Closed (fixed)
Project:
Drupal core
Version:
10.1.x-dev
Component:
phpunit
Priority:
Minor
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
27 Jan 2023 at 07:53 UTC
Updated:
5 Dec 2023 at 08:23 UTC
Jump to comment: Most recent
Comments
Comment #3
volegerCSpell/SortTest shows that the file contains a list that is not sorted correctly.
Adding changes to the list makes the test passing.
Comment #4
volegerComment #5
longwaveThe dictionary generator is supposed to sort the file, but the error proves that we must have merged some changes manually; so the test is good to ensure that it stays sorted.
A few nitpicks in the test but overall this looks great.
Comment #6
spokjeAren't we fighting the symptom here?
Maybe we can trigger a
yarn spellcheck:make-drupal-dictin commit-code-check.sh on all files if dictionary.txt changes?If the the new dictionary is different from the one in the patch/MR then fail and we should be fine?
Comment #7
longwaveyarn spellcheck:make-drupal-dictis quite slow, this test should be quick. I also think there are still some locale issues with the dictionary generator, as the accented characters sometimes change for reasons I don't yet understand, and regenerating on DrupalCI might trip this up further.Comment #8
longwaveHaving just written #7 it strikes me that the issue might be that we need to apply
LC_ALL=Cto thetrcommand as well as thesortcommand.Comment #9
volegerThanks for the review, I addressed all review comments.
Comment #10
longwaveThanks, this looks OK to me and will prevent regressions here in the future.
Comment #11
volegerRebased after #3337327: CSpell dictionary is out of sync
Comment #12
volegerComment #13
catchI was tempted to agree with Spokje that we could skip this step and go straight to #3338155: Run CSpell on all files when CSpell-related files change... but cherry-picks bypass core commit checks, so this should catch situations where we're introducing discrepancies between 10.1.x and 10.0.x (although only post commit so if it happens we'll need to hotfix it).
Committed/pushed to 10.1.x and cherry-picked to 10.0.x, thanks!
Comment #17
jonathan1055 commentedIf all the commits are done, can the MR be closed as it was not actually merged here.