[Bug 42764] New: codespell.t fails after container OS upgrade due to newer codespell dictionary
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Bug ID: 42764 Summary: codespell.t fails after container OS upgrade due to newer codespell dictionary Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Test Suite Assignee: chris@bigballofwax.co.nz Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org When the KTD container OS is upgraded (e.g. Debian Bookworm -> Trixie), two things happen simultaneously that cause xt/author/codespell.t to fail: 1. The OS codename changes, so Koha::Devel::CI::IncrementalRuns finds no history in the ci-results repository for the new codename and falls back to a full scan of all tracked files. 2. The newer OS ships a newer codespell version (installed unversioned from apt in package-config.yaml). Each codespell release expands its misspelling dictionary, so words silently accepted by the old version are now flagged. The combination means the first full scan on a new container OS fails on pre-existing code that was never touched since the old codespell version accepted it. The .codespell-ignore file (currently 20 entries) was developed against older codespell versions and does not cover terms newly flagged by the upgraded version. Test plan: 1. Apply patch 2. Run xt/author/codespell.t inside the new KTD container => All tests pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|chris@bigballofwax.co.nz |martin.renvoize@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|ASSIGNED |Needs Signoff Sponsorship status|--- |Unsponsored -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199869 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199869&action=edit Bug 42764: Update .codespell-ignore for valid technical terms Add words to .codespell-ignore that codespell 2.4.x flags but are valid technical terms, domain vocabulary, or proper names: - checkin: core library circulation term (check-in as noun/verb) - plack: Perl PSGI middleware framework (proper software name) - requestor: COUNTER 5 standard term used in ERM usage statistics - laf: EDIFACT 3-letter segment code - vai/lamba: Unicode script and language names in character set data - aas: acronym used in Cypress test support files - carmel: contributor name in about.tt - prfile: legacy variable abbreviation in C4/Creators/PDF.pm Test plan: 1. Apply patch 2. prove xt/author/codespell.t => Test passes (or only genuine spelling errors remain) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199870 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199870&action=edit Bug 42764: Fix spelling errors across the codebase Correct genuine spelling errors identified by the codespell 2.4.x full scan after the KTD container OS upgrade. Changes are in comments, POD, strings, and template text across 55 files. "SAN Ouest Provence" (the French library co-operative and Koha sponsor) is preserved as a proper noun. Perl copyright lines receive an inline # codespell:ignore annotation; about.tt files (contributor attribution pages) are added to the Koha::Devel::Files codespell exception list so that future uses of "Provence" as a typo for "Province" are still caught in all other files. Notable user-visible fixes: - "submiting" -> "submitting" in authority and bibliographic merge screens - "deleteable" -> "deletable" in patron exception messages - "Acquistions" -> "Acquisitions" in reports dictionary Test plan: 1. Apply patches 2. In the staff interface, go to Cataloguing > Merge records and confirm the submit button reads "Submitting" (not "Submiting") 3. In the staff interface, go to Authorities > Merge and confirm the same correction 4. Attempt to delete a patron with outstanding issues and confirm the error message reads "deletable" (not "deleteable") 5. prove xt/author/codespell.t => All tests pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |critical -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199884 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199884&action=edit Bug 42764: Fix false positives caused by POD letter-code formatting Two patterns in the codebase produce spurious codespell hits because codespell tokenises word fragments out of formatted letter codes: - B<P>rovider in C4/Auth_with_shibboleth.pm produces 'rovider' - T(ransit), W(aiting), F(inished), P(rocessing) in C4/Reserves.pm produce 'ransit', 'aiting', 'inished', 'rocessing' For the shibboleth POD line the bold-letter markup cannot be simplified without losing the formatting intent, so a per-line # codespell:ignore comment is the right suppression. For the Reserves.pm status table the fix is made at source: the notation is changed from X(word) to X (Word) so codespell sees the full English word and no fragment is produced. Because the per-line comment on the shibboleth line now suppresses 'rovider' directly, the global .codespell-ignore entry for that fragment is removed as redundant. Test plan: 1. Apply patch 2. prove xt/author/codespell.t => No new codespell failures -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |pedro.amorim@openfifth.co.u | |k -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #4 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Passing QA here for Jenkins -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |26.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #5 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Thanks everyone! Pushed to main for 26.11! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #6 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199891 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199891&action=edit Bug 42764: Include filename in codespell test output Without a test comment, failures appear as bare 'not ok N' lines with no indication of which file failed or what codespell found. Passing the filename as the third argument to is() means: - Every passing test shows: ok N - path/to/file - Every failing test shows: not ok N - path/to/file Test plan: 1. Apply patch 2. Introduce a deliberate spelling error in any .pl file 3. prove xt/author/codespell.t => Failing line shows the filename and the codespell finding inline, without needing to dig into full console logs Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199892 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199892&action=edit Bug 42764: Fix false positives caused by mixed filetypes in CI failure store The ci-results repo stores all failures from a single test run as one flat JSON array, regardless of whether they came from the pl, tt, or js filetype pass. On the next incremental run, get_files_to_test('pl') reads that entire list -- including any .tt failures -- and calls remove_exceptions with the 'pl' context. Since .tt files only appear in the tt exception list, they survive the filter and get codespell'd in the pl pass where nothing is configured to exclude them. The fix filters stored failures by filetype before applying exceptions, so each pass only re-checks failures that belong to its own type. Test plan: 1. Apply all patches 2. Add a file to the tt codespell exceptions in Koha::Devel::Files that contains a word codespell would flag 3. Introduce a deliberate failure in another file so the ci-results repo records the excepted .tt file as a previous failure 4. On the next incremental run, confirm the excepted .tt file is not re-tested in the pl or js pass 5. prove xt/author/codespell.t => Tests pass; no false positives from cross-filetype re-checks -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199893 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199893&action=edit Bug 42764: Use inline codespell:ignore for SAN-Ouest Provence in templates Rather than excluding the entire about.tt files from codespell scanning, annotate only the specific lines containing the proper noun "Provence" (SAN-Ouest Provence, a French library co-operative and Koha sponsor) with an inline TT comment that codespell recognises as a per-line ignore. This lets codespell continue to check the rest of each file for genuine spelling mistakes, while still catching accidental "Provence" for "Province" in any other template not carrying this annotation. Replaces the file-level exceptions added to Koha::Devel::Files in favour of the more surgical inline approach. Test plan: 1. Apply patch 2. Verify "Provence" on the annotated lines produces no codespell output: codespell -d --ignore-words .codespell-ignore \ koha-tmpl/intranet-tmpl/prog/en/modules/about.tt => no output 3. Verify the rest of about.tt is still checked by introducing a deliberate typo elsewhere in the file and confirming codespell flags it 4. prove xt/author/codespell.t => All tests pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #9 from Jonathan Druart <jonathan.druart@gmail.com> --- Comment on attachment 199892 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199892 Bug 42764: Fix false positives caused by mixed filetypes in CI failure store Review of attachment 199892: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=42764&attachment=199892) ----------------------------------------------------------------- ::: Koha/Devel/CI/IncrementalRuns.pm @@ +140,4 @@
@koha_commit_history; if ($last_build_commit) { @files = @{ from_json( read_file("$self->{git_repo_dir}/$self->{test_dir}/$last_build_commit") ) }; + @files = grep { ( eval { $dev_files->get_filetype($_) } // '' ) eq $filetype } @files;
Why not simply `@files = grep { $dev_files->get_filetype($_) eq $filetype } @files;`? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #199891|0 |1 is obsolete| | Attachment #199892|0 |1 is obsolete| | Attachment #199893|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199895 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199895&action=edit Bug 42764: Include filename in codespell test output Without a test comment, failures appear as bare 'not ok N' lines with no indication of which file failed or what codespell found. Passing the filename as the third argument to is() means: - Every passing test shows: ok N - path/to/file - Every failing test shows: not ok N - path/to/file Test plan: 1. Apply patch 2. Introduce a deliberate spelling error in any .pl file 3. prove xt/author/codespell.t => Failing line shows the filename and the codespell finding inline, without needing to dig into full console logs Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #11 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199896 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199896&action=edit Bug 42764: Fix false positives caused by mixed filetypes in CI failure store The ci-results repo stores all failures from a single test run as one flat JSON array, regardless of whether they came from the pl, tt, or js filetype pass. On the next incremental run, get_files_to_test('pl') reads that entire list -- including any .tt failures -- and calls remove_exceptions with the 'pl' context. Since .tt files only appear in the tt exception list, they survive the filter and get codespell'd in the pl pass where nothing is configured to exclude them. The fix filters stored failures by filetype before applying exceptions, so each pass only re-checks failures that belong to its own type. Test plan: 1. Apply all patches 2. Add a file to the tt codespell exceptions in Koha::Devel::Files that contains a word codespell would flag 3. Introduce a deliberate failure in another file so the ci-results repo records the excepted .tt file as a previous failure 4. On the next incremental run, confirm the excepted .tt file is not re-tested in the pl or js pass 5. prove xt/author/codespell.t => Tests pass; no false positives from cross-filetype re-checks -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #12 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 199897 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199897&action=edit Bug 42764: Use inline codespell:ignore for SAN-Ouest Provence in templates Rather than excluding the entire about.tt files from codespell scanning, annotate only the specific lines containing the proper noun "Provence" (SAN-Ouest Provence, a French library co-operative and Koha sponsor) with an inline TT comment that codespell recognises as a per-line ignore. This lets codespell continue to check the rest of each file for genuine spelling mistakes, while still catching accidental "Provence" for "Province" in any other template not carrying this annotation. Replaces the file-level exceptions added to Koha::Devel::Files in favour of the more surgical inline approach. Test plan: 1. Apply patch 2. Verify "Provence" on the annotated lines produces no codespell output: codespell -d --ignore-words .codespell-ignore \ koha-tmpl/intranet-tmpl/prog/en/modules/about.tt => no output 3. Verify the rest of about.tt is still checked by introducing a deliberate typo elsewhere in the file and confirming codespell flags it 4. prove xt/author/codespell.t => All tests pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #13 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- (In reply to Jonathan Druart from comment #9)
Comment on attachment 199892 [details] [review] Bug 42764: Fix false positives caused by mixed filetypes in CI failure store
Review of attachment 199892 [details] [review]: -----------------------------------------------------------------
::: Koha/Devel/CI/IncrementalRuns.pm @@ +140,4 @@
@koha_commit_history; if ($last_build_commit) { @files = @{ from_json( read_file("$self->{git_repo_dir}/$self->{test_dir}/$last_build_commit") ) }; + @files = grep { ( eval { $dev_files->get_filetype($_) } // '' ) eq $filetype } @files;
Why not simply `@files = grep { $dev_files->get_filetype($_) eq $filetype } @files;`?
Fixed inline.. I was accounting for the die, but on retrospect we want a loud die if a filetype isn't recognised. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Keywords| |additional_work_needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #14 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Schema is out of sync now because of the change to Koha/Schema/Result/ReportsDictionary.pm -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #15 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Created attachment 200014 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200014&action=edit Bug 42764: (QA follow-up): Atomicupate: Fix typo in SQL table comment Patch from commit 33c6619 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #16 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Created attachment 200015 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200015&action=edit Bug 42764: (QA follow-up): Update DB schema Patch from commit 495e0dd -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #17 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Created attachment 200016 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200016&action=edit [DONT PUSH] Bug 42764: dbic Patch from commit 7209d3d -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #18 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- (In reply to Lucas Gass (lukeg) from comment #14)
Schema is out of sync now because of the change to Koha/Schema/Result/ReportsDictionary.pm
Follow ups pushed. Thanks Lucas. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #19 from Jonathan Druart <jonathan.druart@gmail.com> --- There is still a typo (the parenthesis) Koha/Schema/Result/ReportsDictionary.pm:Koha module this definition is for Circulation, Catalog, Patrons, Acquisitions, Accounts) installer/data/mysql/db_revs/260600001.pl: COMMENT 'Koha module this definition is for (Circulation, Catalog, Patrons, Acquisitions, Accounts)' installer/data/mysql/kohastructure.sql: `report_area` varchar(6) DEFAULT NULL COMMENT 'Koha module this definition is for Circulation, Catalog, Patrons, Acquisitions, Accounts)', -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #20 from Jonathan Druart <jonathan.druart@gmail.com> --- 18:36:03 -Koha module this definition is for Circulation, Catalog, Patrons, Acquisitions, Accounts) 18:36:03 +Koha module this definition is for (Circulation, Catalog, Patrons, Acquisitions, Accounts)
From Koha_Main_MariaDB_update/642/consoleFull
-- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #21 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Created attachment 200083 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200083&action=edit Bug 42764: (QA Follow-up): Fix DB update mismatch perl /kohadevbox/misc4dev/run_tests.pl --run-db-compare-only --compare-with v24.05.00 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #22 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Created attachment 200084 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200084&action=edit Bug 42764: (RM Follow-up): typo fix dbic Patch from commit 38f7d53 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200083|0 |1 is obsolete| | Attachment #200084|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #23 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Created attachment 200085 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200085&action=edit Bug 42764: (QA Follow-up): Fix DB update mismatch perl /kohadevbox/misc4dev/run_tests.pl --run-db-compare-only --compare-with v24.05.00 Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 --- Comment #24 from Pedro Amorim (ammopt) <pedro.amorim@openfifth.co.uk> --- Created attachment 200086 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200086&action=edit Bug 42764: (RM Follow-up): typo fix dbic Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42764 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.11.00 |26.11.00,26.05.00 released in| | Status|Pushed to main |Pushed to stable --- Comment #25 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Pushed to 26.05.x for upcoming 26.05.01 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org