Re: [PR] Modernize BWC testing with parameterized tests [lucene]
HoustonPutman commented on PR #13046: URL: https://github.com/apache/lucene/pull/13046#issuecomment-1936231183 So I just finished the 8.11.3 release, and had some problems with the back-compat testing code. Obviously the ant/gradle switch and jdk versions meant that I had to do a lot of it manually, but I'm pretty sure the regex matching in the `update_backcompat_tests()` function in `addBackcompatIndexes.py` was broken by this commit. I updated the files myself manually for 8.11.3 (on main and 9x), and I think they are correct (and the tests pass), but I would recommend checking them and re-checking that the python script works with these changes... -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
s1monw commented on PR #13046: URL: https://github.com/apache/lucene/pull/13046#issuecomment-1919215269 thanks everybody... I will go and backport this to 9.x as well -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
s1monw merged PR #13046: URL: https://github.com/apache/lucene/pull/13046 -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
uschindler commented on code in PR #13046: URL: https://github.com/apache/lucene/pull/13046#discussion_r1471723409 ## lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java: ## @@ -0,0 +1,253 @@ +/* + * 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.backward_index; + +import com.carrotsearch.randomizedtesting.annotations.Name; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.text.ParseException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.apache.lucene.codecs.Codec; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.SegmentReader; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.lucene.tests.util.TestUtil; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.IOUtils; +import org.apache.lucene.util.Version; +import org.junit.After; +import org.junit.Before; + +public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase { + + protected final Version version; + private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion(); + protected final String indexPattern; + protected static final Set BINARY_SUPPORTED_VERSIONS; + + static { +String[] oldVersions = +new String[] { + "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0", + "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1", + "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3", + "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0", + "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2", + "8.11.2", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0", "9.4.1", "9.4.2", "9.5.0", "9.6.0", + "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2", "9.10.0", "10.0.0", +}; + +Set binaryVersions = new HashSet<>(); +for (String version : oldVersions) { + try { +Version v = Version.parse(version); +assertTrue("Unsupported binary version: " + v, v.major >= Version.MIN_SUPPORTED_MAJOR - 1); +binaryVersions.add(v); + } catch (ParseException ex) { +throw new RuntimeException(ex); + } +} +List allCurrentVersions = getAllCurrentVersions(); +for (Version version : allCurrentVersions) { + // make sure we never miss a version. + assertTrue("Version: " + version + " missing", binaryVersions.remove(version)); +} +BINARY_SUPPORTED_VERSIONS = binaryVersions; + } + + /** + * This is a base constructor for parameterized BWC tests. The constructor arguments are provided + * by {@link com.carrotsearch.randomizedtesting.RandomizedRunner} during test execution. A {@link + * com.carrotsearch.randomizedtesting.annotations.ParametersFactory} specified in a subclass + * provides a list lists of arguments for the tests and RandomizedRunner will execute the test for + * each of the argument list. + * + * @param version the version this test should run for + * @param indexPattern an index pattern in order to open an index of see {@link + * #createPattern(String, String)} + */ + protected BackwardsCompatibilityTestBase( + @Name("version") Version version, @Name("pattern") String indexPattern) { +this.version = version; +this.indexPattern = indexPattern; + } + + Directory directory; + + @Override + @Before + public void setUp() throws Exception { +super.setUp(); +assertNull( +"Index name " + version + " should not exist found", +TestAncientIndicesCompatibility.class.getResourceAsStream( +
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
uschindler commented on code in PR #13046: URL: https://github.com/apache/lucene/pull/13046#discussion_r1471716431 ## lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java: ## @@ -0,0 +1,253 @@ +/* + * 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.backward_index; + +import com.carrotsearch.randomizedtesting.annotations.Name; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.text.ParseException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.apache.lucene.codecs.Codec; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.SegmentReader; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.lucene.tests.util.TestUtil; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.IOUtils; +import org.apache.lucene.util.Version; +import org.junit.After; +import org.junit.Before; + +public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase { + + protected final Version version; + private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion(); + protected final String indexPattern; + protected static final Set BINARY_SUPPORTED_VERSIONS; + + static { +String[] oldVersions = +new String[] { + "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0", + "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1", + "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3", + "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0", + "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2", + "8.11.2", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0", "9.4.1", "9.4.2", "9.5.0", "9.6.0", + "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2", "9.10.0", "10.0.0", +}; + +Set binaryVersions = new HashSet<>(); +for (String version : oldVersions) { + try { +Version v = Version.parse(version); +assertTrue("Unsupported binary version: " + v, v.major >= Version.MIN_SUPPORTED_MAJOR - 1); +binaryVersions.add(v); + } catch (ParseException ex) { +throw new RuntimeException(ex); + } +} +List allCurrentVersions = getAllCurrentVersions(); +for (Version version : allCurrentVersions) { + // make sure we never miss a version. + assertTrue("Version: " + version + " missing", binaryVersions.remove(version)); +} +BINARY_SUPPORTED_VERSIONS = binaryVersions; + } + + /** + * This is a base constructor for parameterized BWC tests. The constructor arguments are provided + * by {@link com.carrotsearch.randomizedtesting.RandomizedRunner} during test execution. A {@link + * com.carrotsearch.randomizedtesting.annotations.ParametersFactory} specified in a subclass + * provides a list lists of arguments for the tests and RandomizedRunner will execute the test for + * each of the argument list. + * + * @param version the version this test should run for + * @param indexPattern an index pattern in order to open an index of see {@link + * #createPattern(String, String)} + */ + protected BackwardsCompatibilityTestBase( + @Name("version") Version version, @Name("pattern") String indexPattern) { +this.version = version; +this.indexPattern = indexPattern; + } + + Directory directory; + + @Override + @Before + public void setUp() throws Exception { +super.setUp(); +assertNull( +"Index name " + version + " should not exist found", +TestAncientIndicesCompatibility.class.getResourceAsStream( +
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
uschindler commented on code in PR #13046: URL: https://github.com/apache/lucene/pull/13046#discussion_r1471695464 ## lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java: ## @@ -0,0 +1,253 @@ +/* + * 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.backward_index; + +import com.carrotsearch.randomizedtesting.annotations.Name; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.text.ParseException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.apache.lucene.codecs.Codec; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.SegmentReader; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.lucene.tests.util.TestUtil; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.IOUtils; +import org.apache.lucene.util.Version; +import org.junit.After; +import org.junit.Before; + +public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase { + + protected final Version version; + private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion(); + protected final String indexPattern; + protected static final Set BINARY_SUPPORTED_VERSIONS; + + static { +String[] oldVersions = +new String[] { + "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0", + "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1", + "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3", + "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0", + "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2", + "8.11.2", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0", "9.4.1", "9.4.2", "9.5.0", "9.6.0", + "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2", "9.10.0", "10.0.0", +}; + +Set binaryVersions = new HashSet<>(); +for (String version : oldVersions) { + try { +Version v = Version.parse(version); +assertTrue("Unsupported binary version: " + v, v.major >= Version.MIN_SUPPORTED_MAJOR - 1); +binaryVersions.add(v); + } catch (ParseException ex) { +throw new RuntimeException(ex); + } +} +List allCurrentVersions = getAllCurrentVersions(); +for (Version version : allCurrentVersions) { + // make sure we never miss a version. + assertTrue("Version: " + version + " missing", binaryVersions.remove(version)); +} +BINARY_SUPPORTED_VERSIONS = binaryVersions; + } + + /** + * This is a base constructor for parameterized BWC tests. The constructor arguments are provided + * by {@link com.carrotsearch.randomizedtesting.RandomizedRunner} during test execution. A {@link + * com.carrotsearch.randomizedtesting.annotations.ParametersFactory} specified in a subclass + * provides a list lists of arguments for the tests and RandomizedRunner will execute the test for + * each of the argument list. + * + * @param version the version this test should run for + * @param indexPattern an index pattern in order to open an index of see {@link + * #createPattern(String, String)} + */ + protected BackwardsCompatibilityTestBase( + @Name("version") Version version, @Name("pattern") String indexPattern) { +this.version = version; +this.indexPattern = indexPattern; + } + + Directory directory; + + @Override + @Before + public void setUp() throws Exception { +super.setUp(); +assertNull( +"Index name " + version + " should not exist found", +TestAncientIndicesCompatibility.class.getResourceAsStream( +
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
uschindler commented on PR #13046: URL: https://github.com/apache/lucene/pull/13046#issuecomment-1917572859 There is no formatted() with locale. -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
s1monw commented on PR #13046: URL: https://github.com/apache/lucene/pull/13046#issuecomment-1917508359 @uschindler I need to hear you speaking german to tell, you know 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
uschindler commented on PR #13046: URL: https://github.com/apache/lucene/pull/13046#issuecomment-1917488764 > > > We should PnP this! > > > > > > What on earth means PnP? Mike, check out this search: https://www.google.com/search?q=pnp+acronym wikipedia FTW > > PnP = progress not perfection! Not plug and play, not transistors, heh. We all need some drugs sometimes, especially the policeman. -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
mikemccand commented on PR #13046: URL: https://github.com/apache/lucene/pull/13046#issuecomment-1917387757 > > We should PnP this! > > What on earth means PnP? Mike, check out this search: https://www.google.com/search?q=pnp+acronym wikipedia FTW PnP = progress not perfection! Not plug and play, not transistors, heh. -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
uschindler commented on code in PR #13046: URL: https://github.com/apache/lucene/pull/13046#discussion_r1471018363 ## lucene/backward-codecs/src/test/org/apache/lucene/backward_index/BackwardsCompatibilityTestBase.java: ## @@ -0,0 +1,252 @@ +/* + * 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.backward_index; + +import com.carrotsearch.randomizedtesting.annotations.Name; +import java.io.IOException; +import java.io.InputStream; +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.text.ParseException; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.apache.lucene.codecs.Codec; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.SegmentReader; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.lucene.tests.util.TestUtil; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.IOUtils; +import org.apache.lucene.util.Version; +import org.junit.After; +import org.junit.Before; + +public abstract class BackwardsCompatibilityTestBase extends LuceneTestCase { + + protected final Version version; + private static final Version LATEST_PREVIOUS_MAJOR = getLatestPreviousMajorVersion(); + protected final String indexPattern; + protected static final Set BINARY_SUPPORTED_VERSIONS; + + static { +String[] oldVersions = +new String[] { + "8.0.0", "8.0.0", "8.1.0", "8.1.0", "8.1.1", "8.1.1", "8.2.0", "8.2.0", "8.3.0", "8.3.0", + "8.3.1", "8.3.1", "8.4.0", "8.4.0", "8.4.1", "8.4.1", "8.5.0", "8.5.0", "8.5.1", "8.5.1", + "8.5.2", "8.5.2", "8.6.0", "8.6.0", "8.6.1", "8.6.1", "8.6.2", "8.6.2", "8.6.3", "8.6.3", + "8.7.0", "8.7.0", "8.8.0", "8.8.0", "8.8.1", "8.8.1", "8.8.2", "8.8.2", "8.9.0", "8.9.0", + "8.10.0", "8.10.0", "8.10.1", "8.10.1", "8.11.0", "8.11.0", "8.11.1", "8.11.1", "8.11.2", + "8.11.2", "9.0.0", "9.1.0", "9.2.0", "9.3.0", "9.4.0", "9.4.1", "9.4.2", "9.5.0", "9.6.0", + "9.7.0", "9.8.0", "9.9.0", "9.9.1", "9.9.2", "9.10.0", "10.0.0", +}; + +Set binaryVersions = new HashSet<>(); +for (String version : oldVersions) { + try { +Version v = Version.parse(version); +assertTrue("Unsupported binary version: " + v, v.major >= Version.MIN_SUPPORTED_MAJOR - 1); +binaryVersions.add(v); + } catch (ParseException ex) { +throw new RuntimeException(ex); + } +} +List allCurrentVersions = getAllCurrentVersions(); +for (Version version : allCurrentVersions) { + // make sure we never miss a version. + assertTrue("Version: " + version + " missing", binaryVersions.remove(version)); +} +BINARY_SUPPORTED_VERSIONS = binaryVersions; + } + + /** + * This is a base constructor for parameterized BWC tests. The constructor arguments are provided + * by {@link com.carrotsearch.randomizedtesting.RandomizedRunner} during test execution. A {@link + * com.carrotsearch.randomizedtesting.annotations.ParametersFactory} specified in a subclass + * provides a list lists of arguments for the tests and RandomizedRunner will execute the test for + * each of the argument list. + * + * @param version the version this test should run for + * @param indexPattern an index pattern in order to open an index of see {@link + * #createPattern(String, String)} + */ + protected BackwardsCompatibilityTestBase( + @Name("version") Version version, @Name("pattern") String indexPattern) { +this.version = version; +this.indexPattern = indexPattern; + } + + Directory directory; + + @Override + @Before + public void setUp() throws Exception { +super.setUp(); +assertNull( +"Index name " + version + " should not exist found", +TestAncientIndicesCompatibility.class.getResourceAsStream( +
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
s1monw commented on PR #13046: URL: https://github.com/apache/lucene/pull/13046#issuecomment-1916447395 > We should PnP this! What on earth means PnP? Mike, check out this search: https://www.google.com/search?q=pnp+acronym wikipedia FTW -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
mikemccand commented on code in PR #13046: URL: https://github.com/apache/lucene/pull/13046#discussion_r1469886161 ## lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestIndexUpgradeBackwardsCompatibility.java: ## @@ -0,0 +1,260 @@ +/* + * 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.backward_index; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import org.apache.lucene.document.Document; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexUpgrader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.MergePolicy; +import org.apache.lucene.index.SegmentCommitInfo; +import org.apache.lucene.index.SegmentInfos; +import org.apache.lucene.store.ByteBuffersDirectory; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FSDirectory; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.NIOFSDirectory; +import org.apache.lucene.tests.analysis.MockAnalyzer; +import org.apache.lucene.tests.util.TestUtil; +import org.apache.lucene.util.InfoStream; +import org.apache.lucene.util.Version; + +public class TestIndexUpgradeBackwardsCompatibility extends BackwardsCompatibilityTestBase { + + public TestIndexUpgradeBackwardsCompatibility(Version version, String pattern) { +super(version, pattern); + } + + @ParametersFactory(argumentFormatting = "Lucene-Version:%1$s; Pattern: %2$s") + public static Iterable testVersionsFactory() throws IllegalAccessException { +Iterable allSupportedVersions = +allVersion( +TestBasicBackwardsCompatibility.INDEX_NAME, +TestBasicBackwardsCompatibility.SUFFIX_CFS, +TestBasicBackwardsCompatibility.SUFFIX_NO_CFS); +return allSupportedVersions; + } + + /** Randomizes the use of some of hte constructor variations */ + static IndexUpgrader newIndexUpgrader(Directory dir) { +final boolean streamType = random().nextBoolean(); +final int choice = TestUtil.nextInt(random(), 0, 2); +switch (choice) { + case 0: +return new IndexUpgrader(dir); + case 1: +return new IndexUpgrader(dir, streamType ? null : InfoStream.NO_OUTPUT, false); + case 2: +return new IndexUpgrader(dir, newIndexWriterConfig(null), false); + default: +fail("case statement didn't get updated when random bounds changed"); +} +return null; // never get here + } + + public void testUpgradeOldIndex() throws Exception { +Directory dir = newDirectory(directory); +int indexCreatedVersion = SegmentInfos.readLatestCommit(dir).getIndexCreatedVersionMajor(); +newIndexUpgrader(dir).upgrade(); + +checkAllSegmentsUpgraded(dir, indexCreatedVersion); +dir.close(); + } + + @Override + protected void createIndex(Directory directory) throws IOException { +if (indexPattern.equals( +createPattern( +TestBasicBackwardsCompatibility.INDEX_NAME, +TestBasicBackwardsCompatibility.SUFFIX_CFS))) { + TestBasicBackwardsCompatibility.createIndex(directory, true, false); +} else { + TestBasicBackwardsCompatibility.createIndex(directory, false, false); +} + } + + public void testUpgradeOldSingleSegmentIndexWithAdditions() throws Exception { +// NOCOMMIT we use to have single segment indices but we stopped creating them at some point Review Comment: I don't know why we stopped! We should start again? But maybe as followon? ## lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestIndexUpgradeBackwardsCompatibility.java: ## @@ -0,0 +1,260 @@ +/* + * 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
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
dweiss commented on PR #13046: URL: https://github.com/apache/lucene/pull/13046#issuecomment-1915017690 Oh, one more problem is that you can't "see" the structure of tests before you actually run them (in an IDE). Don't know how much of an issue this is in practice but it's impossible to solve with JUnit4. Some IDEs may also have problems repeating a particular test if its name contains parameters and their names. But I don't think we should think in terms of IDE support - the final outcome and benefit for maintainability matters more. -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
dweiss commented on PR #13046: URL: https://github.com/apache/lucene/pull/13046#issuecomment-1915011660 I like it. Whenever there is test repetition that can be driven by data, it should be driven by data. The only downside to using ParametersFactory is that it's something that is implemented by the randomizedtesting framework (not JUnit itself) and people may be unfamiliar with it - perhaps a comment on the class (or the parameterized constructor) pointing at where the arguments come from would make it easier to grasp how this thing works? Plain JUnit alternatives do exist but they rely on a different test runner (Parameterized) - you wouldn't be able to extend from LuceneTestCase anymore (and you'd be outside of the randomization context). I don't see any other, more elegant, solutions. -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
jpountz commented on code in PR #13046: URL: https://github.com/apache/lucene/pull/13046#discussion_r1469520708 ## lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestBinaryBackwardsCompatibility.java: ## @@ -0,0 +1,87 @@ +/* + * 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.backward_index; + +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexCommit; +import org.apache.lucene.index.IndexFormatTooOldException; +import org.apache.lucene.index.SegmentInfos; +import org.apache.lucene.index.StandardDirectoryReader; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.store.BaseDirectoryWrapper; +import org.apache.lucene.util.Version; + +public class TestBinaryBackwardsCompatibility extends BackwardsCompatibilityTestBase { + + static final int MIN_BINARY_SUPPORTED_MAJOR = Version.MIN_SUPPORTED_MAJOR - 1; + static final String INDEX_NAME = "unsupported"; + static final String SUFFIX_CFS = "-cfs"; + static final String SUFFIX_NO_CFS = "-nocfs"; + + public TestBinaryBackwardsCompatibility(Version version, String pattern) { +super(version, pattern); + } + + @ParametersFactory(argumentFormatting = "Lucene-Version:%1$s; Pattern: %2$s") + public static Iterable testVersionsFactory() { +List params = new ArrayList<>(); +for (Version version : BINARY_SUPPORTED_VERSIONS) { + params.add(new Object[] {version, createPattern(INDEX_NAME, SUFFIX_CFS)}); + params.add(new Object[] {version, createPattern(INDEX_NAME, SUFFIX_NO_CFS)}); +} +return params; + } + + @Override + void verifyUsesDefaultCodec(Directory dir, String name) throws IOException { +// don't this will fail since the indices are not supportd Review Comment: ```suggestion // don't this will fail since the indices are not supported ``` ## lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestEmptyIndexBackwardsCompatibility.java: ## @@ -0,0 +1,65 @@ +/* + * 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.backward_index; + +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.NoMergePolicy; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.analysis.MockAnalyzer; +import org.apache.lucene.tests.util.TestUtil; +import org.apache.lucene.util.Version; + +public class TestEmptyIndexBackwardsCompatibility extends BackwardsCompatibilityTestBase { + static final String INDEX_NAME = "empty"; + static final String SUFFIX = ""; + + public TestEmptyIndexBackwardsCompatibility(Version version, String pattern) { +super(version, pattern); + } + + @Override + protected void createIndex(Directory directory) throws IOException { +IndexWriterConfig conf = +new IndexWriterConfig(new MockAnalyzer(random())) +.setUseCompoundFile(false) +.setCodec(TestUtil.getDefaultCodec()) +.setMergePolicy(NoMergePolicy.INSTANCE); +try (IndexWriter writer = new IndexWriter(directory, conf)) { +
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
s1monw commented on code in PR #13046: URL: https://github.com/apache/lucene/pull/13046#discussion_r1469520535 ## lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestIndexUpgradeBackwardsCompatibility.java: ## @@ -0,0 +1,260 @@ +/* + * 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.backward_index; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.PrintStream; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; +import org.apache.lucene.document.Document; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexUpgrader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.MergePolicy; +import org.apache.lucene.index.SegmentCommitInfo; +import org.apache.lucene.index.SegmentInfos; +import org.apache.lucene.store.ByteBuffersDirectory; +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FSDirectory; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.NIOFSDirectory; +import org.apache.lucene.tests.analysis.MockAnalyzer; +import org.apache.lucene.tests.util.TestUtil; +import org.apache.lucene.util.InfoStream; +import org.apache.lucene.util.Version; + +public class TestIndexUpgradeBackwardsCompatibility extends BackwardsCompatibilityTestBase { + + public TestIndexUpgradeBackwardsCompatibility(Version version, String pattern) { +super(version, pattern); + } + + @ParametersFactory(argumentFormatting = "Lucene-Version:%1$s; Pattern: %2$s") + public static Iterable testVersionsFactory() throws IllegalAccessException { +Iterable allSupportedVersions = +allVersion( +TestBasicBackwardsCompatibility.INDEX_NAME, +TestBasicBackwardsCompatibility.SUFFIX_CFS, +TestBasicBackwardsCompatibility.SUFFIX_NO_CFS); +return allSupportedVersions; + } + + /** Randomizes the use of some of hte constructor variations */ + static IndexUpgrader newIndexUpgrader(Directory dir) { +final boolean streamType = random().nextBoolean(); +final int choice = TestUtil.nextInt(random(), 0, 2); +switch (choice) { + case 0: +return new IndexUpgrader(dir); + case 1: +return new IndexUpgrader(dir, streamType ? null : InfoStream.NO_OUTPUT, false); + case 2: +return new IndexUpgrader(dir, newIndexWriterConfig(null), false); + default: +fail("case statement didn't get updated when random bounds changed"); +} +return null; // never get here + } + + public void testUpgradeOldIndex() throws Exception { +Directory dir = newDirectory(directory); +int indexCreatedVersion = SegmentInfos.readLatestCommit(dir).getIndexCreatedVersionMajor(); +newIndexUpgrader(dir).upgrade(); + +checkAllSegmentsUpgraded(dir, indexCreatedVersion); +dir.close(); + } + + @Override + protected void createIndex(Directory directory) throws IOException { +if (indexPattern.equals( +createPattern( +TestBasicBackwardsCompatibility.INDEX_NAME, +TestBasicBackwardsCompatibility.SUFFIX_CFS))) { + TestBasicBackwardsCompatibility.createIndex(directory, true, false); +} else { + TestBasicBackwardsCompatibility.createIndex(directory, false, false); +} + } + + public void testUpgradeOldSingleSegmentIndexWithAdditions() throws Exception { +// NOCOMMIT we use to have single segment indices but we stopped creating them at some point Review Comment: maybe somebody has an answer to that, this puzzles me as well -- 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To
Re: [PR] Modernize BWC testing with parameterized tests [lucene]
s1monw commented on code in PR #13046: URL: https://github.com/apache/lucene/pull/13046#discussion_r1469519342 ## lucene/backward-codecs/src/test/org/apache/lucene/backward_index/TestDVUpdateBackwardsCompatibility.java: ## @@ -0,0 +1,268 @@ +/* + * 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.backward_index; + +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.apache.lucene.document.BinaryDocValuesField; +import org.apache.lucene.document.Document; +import org.apache.lucene.document.Field; +import org.apache.lucene.document.NumericDocValuesField; +import org.apache.lucene.document.StringField; +import org.apache.lucene.index.BinaryDocValues; +import org.apache.lucene.index.DirectoryReader; +import org.apache.lucene.index.IndexWriter; +import org.apache.lucene.index.IndexWriterConfig; +import org.apache.lucene.index.LeafReader; +import org.apache.lucene.index.LeafReaderContext; +import org.apache.lucene.index.NoMergePolicy; +import org.apache.lucene.index.NumericDocValues; +import org.apache.lucene.index.PostingsEnum; +import org.apache.lucene.index.Term; +import org.apache.lucene.index.TermsEnum; +import org.apache.lucene.search.DocIdSetIterator; +import org.apache.lucene.store.Directory; +import org.apache.lucene.tests.analysis.MockAnalyzer; +import org.apache.lucene.tests.util.TestUtil; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.Version; + +public class TestDVUpdateBackwardsCompatibility extends BackwardsCompatibilityTestBase { + + static final String INDEX_NAME = "dvupdates"; + static final String SUFFIX = ""; + + public TestDVUpdateBackwardsCompatibility(Version version, String pattern) { +super(version, pattern); + } + + @ParametersFactory(argumentFormatting = "Lucene-Version:%1$s; Pattern: %2$s") + public static Iterable testVersionsFactory() { +List params = new ArrayList<>(); +// NOCOMMIT: why are we only testing one version here? Review Comment: maybe somebody has an answer to 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. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org