You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/01/29 09:33:07 UTC
[GitHub] [lucene-solr] donnerpeter opened a new pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
donnerpeter opened a new pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267
and unify test naming in SpellCheckerTest
<!--
_(If you are a project committer then you may remove some/all of the following template.)_
Before creating a pull request, please file an issue in the ASF Jira system for Lucene or Solr:
* https://issues.apache.org/jira/projects/LUCENE
* https://issues.apache.org/jira/projects/SOLR
You will need to create an account in Jira in order to create an issue.
The title of the PR should reference the Jira issue number in the form:
* LUCENE-####: <short description of problem or changes>
* SOLR-####: <short description of problem or changes>
LUCENE and SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
# Description
We want to have more or less parity in Java and C++ Hunspell implementations
# Solution
See below
# Tests
Added `TestsFromOriginalHunspellRepository` that runs spell checker on all flies from Hunspell repository which should be checked out somewhere. Some tests still fail there, but it won't fail on the CI now because it only runs when there's a special system property defined.
# Checklist
Please review the following and check all that apply:
- [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
- [x] I have created a Jira issue and added the issue ID to my pull request title.
- [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
- [x] I have developed this patch against the `master` branch.
- [x] I have run `./gradlew check`.
- [x] I have added tests for my changes.
- [ ] I have added documentation for the [Ref Guide](https://github.com/apache/lucene-solr/tree/master/solr/solr-ref-guide) (for Solr changes only).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#issuecomment-771503328
> Eh. This is why Parameterized works for you and randomizedtesting doesn't:
> https://github.com/JetBrains/intellij-community/blob/master/plugins/junit_rt/src/com/intellij/junit4/JUnit4TestRunnerUtil.java#L96-L105
That's what I feared: relying on JUnit internals :(
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567714151
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
Yes, I've merged it with the master and renamed tests. The last commit changes the parameterization to the recommended, and that's where I get that behavior :(
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568176360
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
Filed an issue for myself here: https://github.com/randomizedtesting/randomizedtesting/issues/295.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568402121
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
Thanks! Now I'm starting to doubt whether this approach makes sense at all. I could avoid parameterization by generating test methods explicitly by files, with some risk that new files appear (which could be checked by additional code).
And is it OK to modify the test policy for such local tests? I planned to add more not-easy-to-have-in-CI tests, which would measure performance and check correctness. They'd need external files with dictionaries, corpora for various languages (external or is there anything internal already?), and a test-only Hunspell JNI library for comparison (which needs a native binary and a couple of other jars, all of them need sha and license files, and it all gets quite verbose). Do you think the benefits of having this in the repo outweigh the costs? I could also leave this all locally, since I seem to be the only one needing these tests in the near future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567675910
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
If you leave this behind, it'll just be there forever... I can convert it if you wish. And I think it does make sense to maybe run these tests on a CI at some point. Just to make sure no regressions occur.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568394955
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Thanks for looking into this and for your patch! I've no idea why you can't push, I've got the checkbox enabled on this PR:
That's what https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork seems to recommend. I also checked the repository settings and couldn't find an option about committers. I can invite you personally though :)
By renaming do you mean `TestSpellChecker`? I'll do it, thanks for the suggestion, but preferably a bit later, when there won't be so much merging around this class :)
BTW what do you think about renaming `SpellChecker` into `Hunspell`?
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
Thanks! Now I'm starting to doubt whether this approach makes sense at all. I could avoid parameterization by generating test methods explicitly by files, with some risk that new files appear (which could be checked by additional code).
And is it OK to modify the test policy for such local tests? I planned to add more not-easy-to-have-in-CI tests, which would measure performance and check correctness. They'd need external files with dictionaries, corpora for various languages (external or is there anything internal already?), and a test-only Hunspell JNI library for comparison (which needs a native binary and a couple of other jars, all of them need sha and license files, and it all gets quite verbose). Do you think the benefits of having this in the repo outweigh the costs? I could also leave this all locally, since I seem to be the only one needing these tests in the near future.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567708785
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
This is now a method reused in the new test which deals with some random path on the disk, unrelated to resources.
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
This is now a method reused in the new test which deals with some random path on the disk, unrelated to classpath resources.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567677305
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
I'll deal with it. Just asking :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#issuecomment-771498850
Eh. This is why Parameterized works for you and randomizedtesting doesn't:
https://github.com/JetBrains/intellij-community/blob/master/plugins/junit_rt/src/com/intellij/junit4/JUnit4TestRunnerUtil.java#L96-L105
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568415196
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
I think they should reside in the repo if they are useful (even for local launches). What I'm afraid of is that if these tools are not in use, they'll eventually degrade and stop working without anyone noticing.
I think the way to integrate such tests properly would be to add a specific gradle test task which would configure an appropriate policy, require pointers to the required resources, etc. This way these tests can be run as a CI run (somewhere... maybe a github action, even?).
I think this can be ironed out later on, once you've written (notice the 'you' here... ;) more of such tests - the patterns of making them work with the CI will naturally emerge from that.
For now, feel free to use that original parameterized test runner - I'll look into making IntelliJ work with randomizedtesting again (because I use it here and in other projects). It's a moving target and thus a bit discouraging (I did the same thing a few times in the past already for various IDEs that interpreted test descriptions differently).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567663562
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Is there any utility to read all lines from an input stream conveniently, or should I use the plain old `BufferedReader`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567700660
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
Test names are essentially Descriptions in JUnit. Filtering by the same test description should re-run the same test. IntelliJ's behavior has been changing over time here and I really don't know why it's not working now... Do you have it on that branch? I'll take a look (later).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568186778
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
You can't really convert resource URLs to paths with url.getPath. This breaks, as I suspected. On Windows you get:
```
java.nio.file.InvalidPathException: Illegal char <:> at index 2: /C:/Work/apache/lucene/lucene.master/lucene/analysis/common/build/classes/java/test/org/apache/lucene/analysis/hunspell/i53643.aff
> at __randomizedtesting.SeedInfo.seed([FE61D482FAEDBB53:CE18D8B46A2785A8]:0)
> at java.base/sun.nio.fs.WindowsPathParser.normalize(WindowsPathParser.java:182)
> at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:153)
> at java.base/sun.nio.fs.WindowsPathParser.parse(WindowsPathParser.java:77)
> at java.base/sun.nio.fs.WindowsPath.parse(WindowsPath.java:92)
> at java.base/sun.nio.fs.WindowsFileSystem.getPath(WindowsFileSystem.java:229)
> at java.base/java.nio.file.Path.of(Path.java:147)
```
A better method is to go through the URI - Path.of(url.toUri()). I've modified the code slightly, please take a look.
Also, can you rename tests to follow TestXXX convention? This may be enforced in the future and will spare somebody some work to rename.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568175124
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
I checked intellij and parameterized tests tonight. It's what I was afraid of - test descriptions are emitted correctly (in my opinion) but they're *interepreted* differently depending on the tool (and time when you check...).
The reason why you see the class name and test method before each actual test is because these supposedly "hidden" elements allowed tools to go back to the source code of a test with an arbitrary name (if you double-click on a test in IntelliJ it will take you back to the test method). Relaunching of a single test must have changed at some point because it used to be an exact name filter... but now I it just reruns all tests under a test method (all parameter variations).
It's worth mentioning that this isn't consistent even in IntelliJ itself - if I run a simple(r) parameterized test via IntelliJ launcher, I get this test suite tree:
But when I run the same test via gradle launcher (from within the IDE), I get this tree:
I don't know if there is a way to make all the tools happy; test descriptions and nesting is broken in JUnit 4.x itself.
Given the above, please feel free to revert back to what works for you. I'd name the test class TestHunspellRepositoryTestCases for clarity. Also, this test will not run under Lucene test framework because the security manager won't let you access arbitrary paths outside the build location. You'd need to add this to tests.policy:
```
permission java.io.FilePermission "${hunspell.repo.path}${/}-", "read";
```
Don't know whether it's worth it at the moment though.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#issuecomment-771504446
Yes, sadly. I haven't looked at junit5, shame on me. Perhaps it's improved there.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#issuecomment-771492242
I've done some rebasing, included your patch, renamed the test and tweaked the code a bit. Hopefully it's better now.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568398245
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Renaming SpellChecker to Hunspell - yes, I think it's a good idea. Renaming tests later - absolutely.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567680987
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
You can also assumeTrue(false) (throw an assumption exception) if the property is not defined. This will mark the test as ignored rather than empty?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567689855
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
URLs aren't very friendly, it's hard to check if they exist (probably with try-catch, but that's ugly). Can resources in tests ever be not in the local file system?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567652699
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
Review comment:
The test runner in Lucene will include any methods starting with "test" so you don't have to annotate them, unless you want to.
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Can you change it back to resource stream based implementations? Resource paths are not required to be on the default file system and then this code would fail with an odd exception. Pass the name of the resource and derive resource streams from that.
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
Please run with the default runner (extend RandomizedTest or, better, LuceneTestCase) - it adds a number of extra checks to make sure no interference between tests, proper reporting, etc. If you wish to parameterize via constructor, you still can - an example is here:
https://github.com/randomizedtesting/randomizedtesting/blob/master/examples/maven/src/main/java/com/carrotsearch/examples/randomizedrunner/Test007ParameterizedTests.java
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -125,17 +140,17 @@ protected void doTest(String name) throws Exception {
IOUtils.closeWhileHandlingException(dictStream);
}
- URL good = StemmerTestBase.class.getResource(name + ".good");
- if (good != null) {
- for (String word : Files.readAllLines(Path.of(good.toURI()))) {
Review comment:
I see this was incorrect before as well. Worth correcting though.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567698107
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Why is it hard? getResource returns null if a resource doesn't exist. getResourceAsStream returns null too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567716954
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
Thanks. I'll take a look later, Peter. Will get back to you.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567661539
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
Review comment:
Should I rename all test methods to `testSmth` then?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567662478
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
When can resources in tests be not in the local file system?
Do you mean passing an URL instead?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#issuecomment-771499577
If you take a look at that class you'll understand why it's such a mess to try to navigate those test descriptions...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568397883
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Mhmm... let me try again then. It's weird - tried last night and got permission denied. Could be that I pulled your changes via https and not ssh... Sorry, it was late.
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Renaming SpellChecker to Hunspell - yes, I think it's a good idea. Renaming tests later - absolutely.
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
I think they should reside in the repo if they are useful (even for local launches). What I'm afraid of is that if these tools are not in use, they'll eventually degrade and stop working without anyone noticing.
I think the way to integrate such tests properly would be to add a specific gradle test task which would configure an appropriate policy, require pointers to the required resources, etc. This way these tests can be run as a CI run (somewhere... maybe a github action, even?).
I think this can be ironed out later on, once you've written (notice the 'you' here... ;) more of such tests - the patterns of making them work with the CI will naturally emerge from that.
For now, feel free to use that original parameterized test runner - I'll look into making IntelliJ work with randomizedtesting again (because I use it here and in other projects). It's a moving target and thus a bit discouraging (I did the same thing a few times in the past already for various IDEs that interpreted test descriptions differently).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567697917
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
I'll check if assume works in parameters method, thanks!
Unfortunately randomized+parameterized doesn't play nicely with IntelliJ. First, the presentation is pretty verbose:
vs
Second, more importantly, it's impossible to right-click a single test and run it separately, while that works with the standard parameterized runner. Are you aware of ways to improve that?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#issuecomment-771492242
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568187636
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Ah... can't push to your repo (there is a checkbox to enable committers to do so - please use it, makes edits easier :). Here is the commit:
https://github.com/dweiss/lucene-solr/commit/618a2d3b5bb51eb0e35322a9c56b97bdce7d728b
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567673956
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
Review comment:
Up to you. All I'm saying is you can write testFoo() and it'll run as a test... if you're subclassing LuceneTestCase, that is (which I suggested elsewhere you should be doing - it really helps to keep test runs sane, even if it adds some overhead).
https://github.com/apache/lucene-solr/blob/master/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java#L188
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568394955
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Thanks for looking into this and for your patch! I've no idea why you can't push, I've got the checkbox enabled on this PR:
That's what https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/allowing-changes-to-a-pull-request-branch-created-from-a-fork seems to recommend. I also checked the repository settings and couldn't find an option about committers. I can invite you personally though :)
By renaming do you mean `TestSpellChecker`? I'll do it, thanks for the suggestion, but preferably a bit later, when there won't be so much merging around this class :)
BTW what do you think about renaming `SpellChecker` into `Hunspell`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567674247
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
BufferedReader is the way to go!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss merged pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss merged pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss merged pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss merged pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567674543
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
(don't know if any utility method like that exists).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] donnerpeter commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
donnerpeter commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567664396
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/TestsFromOriginalHunspellRepository.java
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.lucene.analysis.hunspell;
+
+import java.io.IOException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.text.ParseException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+/**
+ * Same as {@link SpellCheckerTest}, but checks all Hunspell's test data. The path to the checked
+ * out Hunspell repository should be in {@code -Dhunspell.repo.path=...} system property.
+ */
+@RunWith(Parameterized.class)
Review comment:
Since this test isn't intended to be run on CI (at least for now), I thought it's safe to skip those. Apparently it's not? :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#issuecomment-771498850
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r568397883
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Mhmm... let me try again then. It's weird - tried last night and got permission denied. Could be that I pulled your changes via https and not ssh... Sorry, it was late.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] dweiss commented on a change in pull request #2267: LUCENE-9707: Hunspell: check Lucene's implementation against Hunspel's test data
Posted by GitBox <gi...@apache.org>.
dweiss commented on a change in pull request #2267:
URL: https://github.com/apache/lucene-solr/pull/2267#discussion_r567712828
##########
File path: lucene/analysis/common/src/test/org/apache/lucene/analysis/hunspell/SpellCheckerTest.java
##########
@@ -61,59 +61,74 @@ public void needAffixOnAffixes() throws Exception {
doTest("needaffix5");
}
+ @Test
public void testBreak() throws Exception {
doTest("break");
}
- public void testBreakDefault() throws Exception {
+ @Test
+ public void breakDefault() throws Exception {
doTest("breakdefault");
}
- public void testBreakOff() throws Exception {
+ @Test
+ public void breakOff() throws Exception {
doTest("breakoff");
}
- public void testCompoundrule() throws Exception {
+ @Test
+ public void compoundrule() throws Exception {
doTest("compoundrule");
}
- public void testCompoundrule2() throws Exception {
+ @Test
+ public void compoundrule2() throws Exception {
doTest("compoundrule2");
}
- public void testCompoundrule3() throws Exception {
+ @Test
+ public void compoundrule3() throws Exception {
doTest("compoundrule3");
}
- public void testCompoundrule4() throws Exception {
+ @Test
+ public void compoundrule4() throws Exception {
doTest("compoundrule4");
}
- public void testCompoundrule5() throws Exception {
+ @Test
+ public void compoundrule5() throws Exception {
doTest("compoundrule5");
}
- public void testCompoundrule6() throws Exception {
+ @Test
+ public void compoundrule6() throws Exception {
doTest("compoundrule6");
}
- public void testCompoundrule7() throws Exception {
+ @Test
+ public void compoundrule7() throws Exception {
doTest("compoundrule7");
}
- public void testCompoundrule8() throws Exception {
+ @Test
+ public void compoundrule8() throws Exception {
doTest("compoundrule8");
}
- public void testGermanCompounding() throws Exception {
+ @Test
+ public void germanCompounding() throws Exception {
doTest("germancompounding");
}
protected void doTest(String name) throws Exception {
- InputStream affixStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".aff"), name);
- InputStream dictStream =
- Objects.requireNonNull(getClass().getResourceAsStream(name + ".dic"), name);
+ checkSpellCheckerExpectations(
Review comment:
Well, this isn't too fortunate then. I'll take a look later and maybe come up with more concrete suggestions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org