LUCENE-9727: build side support for running Hunspell tests. #2313
Conversation
|
@donnerpeter I've done some minor edits so that those checks run via Gradle too. Can you take a peek? I made one change that may affect you - those dictionaries are now scanned recursively without the depth limit - this helps me to run against both repos at once. |
| * documentation} | ||
| * Loads all dictionaries from the directory specified in {@code hunspell.dictionaries} system | ||
| * property and prints their memory usage. All *.aff files are traversed directly inside the given | ||
| * directory or in its immediate subdirectories. Each *.aff file must have a same-named sibling |
There was a problem hiding this comment.
Not only immediate now?
| int threads = Runtime.getRuntime().availableProcessors(); | ||
| ExecutorService executor = Executors.newFixedThreadPool(threads); | ||
| try { | ||
| Deque<Path> failures = new ConcurrentLinkedDeque<>(); |
There was a problem hiding this comment.
synchronizedList might look less surprising to readers :)
There was a problem hiding this comment.
Doesn't matter it a test, but sure.
| + "suffixes=" | ||
| + RamUsageTester.humanSizeOf(dic.suffixes) | ||
| + ")"; | ||
| + ("words=" + RamUsageTester.humanSizeOf(dic.words) + ", ") |
There was a problem hiding this comment.
A nice trick to save on LOCs in the face of this ruthless spotless formatter :)
There was a problem hiding this comment.
Yes, such hints and groupings make logical sense and also help with formatting.
There was a problem hiding this comment.
@donnerpeter I've done some minor edits so that those checks run via Gradle too. Can you take a peek? I made one change that may affect you - those dictionaries are now scanned recursively without the depth limit - this helps me to run against both repos at once.
Thank you very much, LGTM overall! Do you check out both repos into some common parent directory? Looks like a nice idea.
| System.err.println("While checking " + aff + ":"); | ||
| e.printStackTrace(); | ||
| int threads = Runtime.getRuntime().availableProcessors(); | ||
| ExecutorService executor = Executors.newFixedThreadPool(threads); |
There was a problem hiding this comment.
Thanks for parallelization! I've considered it, but never got to do it.
| @@ -40,8 +42,15 @@ | |||
| * en.txt}) in a directory specified in {@code -Dhunspell.corpora=...} | |||
| */ | |||
| @TestCaseOrdering(TestCaseOrdering.AlphabeticOrder.class) | |||
| @Ignore("enable manually") | |||
There was a problem hiding this comment.
I actually took advantage of this. Running all tests in a package skipped the ignored ones, and so the correctness was checked relatively quickly. Now that'd also include performance/allDict tests, and can become slower :( But I can work around that locally.
There was a problem hiding this comment.
Not if you create a launch config that just has one of those properties (just the hunspell.dictionaries) - then perf tests would be skipped. If you launch everything from the same launcher then indeed this will be a problem. I don't care much but it'd be my preference to control it somehow without modifying the code itself. You could use a custom property, test group annotation... whatever works. I think two properties already separate these tests well enough though.
There was a problem hiding this comment.
Sure, that's the way to go
Yes, that's what I did. I just wanted a superset of all the dictionaries... Perhaps eventually make it a gradle task to fetch them automatically and then run integration tests on them. Just goofing around with the code though - I'm not trying to step in your way! |
… directive offsets.
No worries, and thanks :) |
No description provided.