-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Repeatable hunspell tests #14246
Repeatable hunspell tests #14246
Conversation
-Ptests.hunspell.libreoffice.ref=master | ||
-Ptests.hunspell.woorm.ref=main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we pick the 'latest' commit from both branches for the scheduled workflow.
def dictionaryProjects = [ | ||
[ | ||
"libreoffice": "https://github.com/LibreOffice/dictionaries", | ||
"woorm": "https://github.com/wooorm/dictionaries" | ||
].each { name, repo -> | ||
if (!file("${checkoutDir}/${name}").exists()) { | ||
checkoutDir.mkdirs() | ||
// This will work only if git is available, but I assume it is. | ||
project.exec { | ||
"name": "libreoffice", | ||
"url": "https://github.com/LibreOffice/dictionaries", | ||
"ref": "762abe74008b94b2ff06db6f4024b59a8254c467" // head: master | ||
], | ||
[ | ||
"name": "woorm", | ||
"url": "https://github.com/wooorm/dictionaries", | ||
"ref": "8cfea406b505e4d7df52d5a19bce525df98c54ab" // head: main | ||
] | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to be updated by hand, occasionally. The frequency of changes in those repositories isn't that great so no problem there, I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, honestly any revision will work for the intended purpose of preventing regressions. If we want to discover new and interesting ways that dictionaries can be malformed in the wild that we should try to detect, that is different :)
opens org.apache.lucene.util.fst to | ||
org.apache.lucene.test_framework; | ||
opens org.apache.lucene.store to | ||
org.apache.lucene.test_framework; | ||
opens org.apache.lucene.util.automaton to | ||
org.apache.lucene.test_framework; | ||
opens org.apache.lucene.util to | ||
org.apache.lucene.test_framework; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had to add these to make ram testing happy now that the tests run in module mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thank you so much. I'll keep watch on the new weekly builds and meanwhile we'll prevent regressions in PRs without noise!
Thanks. I'll backport to 10x as well. |
Fixes #14235
Sorry this took surprisingly long. I've tried to clean up a few things along the way:
-Ptests.hunspell.libreoffice.ref=master
will test against the master branch (or any other sha provided there). For woorm, it's-Ptests.hunspell.woorm.ref=main
.I also cleaned up gradle scripts a bit.
-Ptests.hunspell.regressions=true
. The separate task wasn't configured properly (it wasn't running in module mode) and in general I think it's better to just keep the 'test' task for running tests.TestAllDictionaries
class, not everything.I also had to open up some core packages to the test framework - this is a side effect of TestAllDictionaries measuring memory of Dictionary objects it creates. I think it'd be nice to make Dictionary implement Accountable and remove opens statements but it's probably a follow up issue.