[Bug 16003] New: Add spelling exclusion patch to QA Test tools
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 Bug ID: 16003 Summary: Add spelling exclusion patch to QA Test tools Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: gmcharlt@gmail.com Reporter: mtompset@hotmail.com QA Contact: testopia@bugs.koha-community.org isnt() is a valid test function, but codespell thinks it is a typo. This adds a -s switch to the qa test tools, to skip the spelling tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 --- Comment #1 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48765 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48765&action=edit Add spelling exclusion parameter -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff Assignee|gmcharlt@gmail.com |mtompset@hotmail.com --- Comment #2 from M. Tompsett <mtompset@hotmail.com> --- It's not really patch 3/3, but I have a couple other tweaks outstanding. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Component|Architecture, internals, |QA tools |and plumbing | Product|Koha |Project Infrastructure Version|master |unspecified CC| |chris@bigballofwax.co.nz QA Contact|testopia@bugs.koha-communit | |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- use of debug statements, marked as failed QA. I would not skip this pass, why do you want? It's certainly better to fix the annoying false positive 'isnt' than completely skip the spelling check. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48765|0 |1 is obsolete| | --- Comment #4 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48782 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48782&action=edit Bug 16003: Add spelling exclusion switch. TEST PLAN --------- Run on a test file with isnt. -- spelling error Patch Run on the same file with a -s -- spelling tests skipped. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #5 from M. Tompsett <mtompset@hotmail.com> --- (In reply to Jonathan Druart from comment #3)
use of debug statements, marked as failed QA.
DOH! Valid point.
I would not skip this pass, why do you want?
Because I rather see SKIPPED than FAILED, when there is a false positive. Additionally, adding the -s is more typing. Most people won't, like they shouldn't.
It's certainly better to fix the annoying false positive 'isnt' than completely skip the spelling check.
I concede that is the better solution. However, since I don't know how to do that, I did what I know how to do as of this moment. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 --- Comment #6 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48791 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48791&action=edit Bug 16003 - Add exceptions file logic to codespell call I created an exceptions file by: $ git grep "isnt(" | cut -f2- -d":" | sort -u > ~/qa-test-tools/spelling.exceptions I then patched QohA/File.pm to include a -x extension to the codespell call if the exceptions file existed. TEST PLAN --------- In your koha development directory apply bug 15870 run the koha qa test tools -- you will get a complaint about isnt(, likely because it is tokenized as isnt. apply this patch to your qa test tools run the koha qa test tools on bug 15870 again -- spelling issue passed! THIS IS A COUNTER PATCH. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 48839 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48839&action=edit Bug 16003: Add exception pattern list for codespell checks -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Mark, Please have a look at this patch. I think it does what you want but in a more flexible way: we won't blacklist an entire file, but just a line. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 M. Tompsett <mtompset@hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #48791|0 |1 is obsolete| | Attachment #48839|0 |1 is obsolete| | --- Comment #9 from M. Tompsett <mtompset@hotmail.com> --- Created attachment 48864 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=48864&action=edit [SIGNED-OFF] Bug 16003: Add exception pattern list for codespell checks The 'isnt' Test::More subroutine should not be considered as a failure by the codespell check. To avoid that this patch introduces the concept of "exception patterns" for this check. NOTE: This is better than introducing a '-x {exception file}', because it dynamically handles the isnt( issue. If other similar function name issues arise in the future, modifications to the i_dont_fail_spelling.pl file and the exception list can be made. Signed-off-by: Mark Tompsett <mtompset@hotmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16003 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Signed Off |RESOLVED --- Comment #10 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed, thanks Mark for the signoff. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org