[GitHub] [lucene-solr] dweiss commented on a change in pull request #1796: LUCENE-9475: Enhance the Gradle build as necessary after removing Ant…

[GitBox <gi...@apache.org>]

dweiss commented on a change in pull request #1796:
URL: https://github.com/apache/lucene-solr/pull/1796#discussion_r479496521



##########
File path: lucene/analysis/common/src/java/org/apache/lucene/analysis/util/UnicodeProps.java
##########
@@ -1,4 +1,5 @@
 // DO NOT EDIT THIS FILE! Use "ant unicode-data" to recreate.
+//nocommit do we have such a target?

Review comment:
       I don't think so. If we don't then did we miss it somehow?

##########
File path: solr/core/src/java/org/apache/solr/handler/export/ExportWriter.java
##########
@@ -210,7 +210,7 @@ public void write(OutputStream os) throws IOException {
     // That causes the totalHits and export entries in the context to _not_ get set.
     // The only time that really matters is when we search against an _empty_ set. That's too obscure
     // a condition to handle as part of this patch, if someone wants to pursue it it can be reproduced with:
-    // ant test  -Dtestcase=StreamingTest -Dtests.method=testAllValidExportTypes -Dtests.seed=10F13879D0D1D6AD -Dtests.slow=true -Dtests.locale=es-PA -Dtests.timezone=America/Bahia_Banderas -Dtests.asserts=true -Dtests.file.encoding=ISO-8859-1
+    // gradlew test  --tests *testAllValidExportTypes* -Ptests.seed=10F13879D0D1D6AD -Ptests.slow=true -Ptests.locale=es-PA -Ptests.timezone=America/Bahia_Banderas -Ptests.asserts=true -Ptests.file.encoding=ISO-8859-1

Review comment:
       I think this should be a qualified class.method reference (much like I showed above).

##########
File path: lucene/backward-codecs/src/test/org/apache/lucene/index/TestManyPointsInOldIndex.java
##########
@@ -36,7 +36,7 @@
 //
 // Compile:
 //   1) temporarily remove 'extends LuceneTestCase' above (else java doesn't see our static void main)

Review comment:
       I'd look as to what this class actually does... seems weird. Classpath below (under "run") is wrong for gradle.

##########
File path: lucene/backward-codecs/src/test/org/apache/lucene/index/TestBackwardsCompatibility.java
##########
@@ -168,7 +169,7 @@ public void testCreateMoreTermsIndex() throws Exception {
     Thread.sleep(100000);
   }
 
-  // ant test -Dtestcase=TestBackwardsCompatibility -Dtestmethod=testCreateSortedIndex -Dtests.codec=default -Dtests.useSecurityManager=false -Dtests.bwcdir=/tmp/sorted
+  // gradlew -p lucene/backward-codecs test --tests *testCreateSortedIndex* -Ptests.bwcdir=/tmp/sorted -Ptests.codec=default -Ptests.useSecurityManager=false

Review comment:
       should be --tests TestBackwardsCompatibility.testCreateSortedIndex.
   A prefix wildcard may be needed (*TestBackwards...)? 

##########
File path: lucene/analysis/kuromoji/src/test/org/apache/lucene/analysis/ja/dict/TokenInfoDictionaryTest.java
##########
@@ -35,6 +35,7 @@
 
 import static org.apache.lucene.analysis.ja.dict.BinaryDictionary.ResourceScheme;
 
+//nocommit

Review comment:
       "test-tools" shouldn't be a task. I don't know what it was doing but it should be a regular test, perhaps grouped under a test group (and enabled by a property).

##########
File path: solr/test-framework/src/java/org/apache/solr/SolrTestCase.java
##########
@@ -130,6 +131,7 @@ public static void checkSyspropForceBeforeClassAssumptionFailure() {
    */
   @Before
   public void checkSyspropForceBeforeAssumptionFailure() {
+    // nocommit, don't see an equivalent Gradle parameter

Review comment:
       I think -Ptests.jvmargs=-Dtests.force...=true should work (didn't check).

##########
File path: lucene/core/src/test/org/apache/lucene/index/Test2BPoints.java
##########
@@ -31,7 +31,7 @@
 import com.carrotsearch.randomizedtesting.annotations.TimeoutSuite;
 
 // e.g. run like this: ant test -Dtestcase=Test2BPoints -Dtests.nightly=true -Dtests.verbose=true -Dtests.monster=true
-// 
+// nocommit, I don't see how to set the heap size, set in gradle.properties? Set to 4G?

Review comment:
       -Ptests.heapsize=6g

##########
File path: lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java
##########
@@ -459,6 +459,7 @@
   public static final boolean LEAVE_TEMPORARY;
   static {
     boolean defaultValue = false;
+    //nocommit

Review comment:
       You can remove reference to ant, but keep the rest.

##########
File path: solr/core/src/test/org/apache/solr/search/facet/SpatialHeatmapFacetsTest.java
##########
@@ -39,6 +39,7 @@ public static void beforeSuperClass() throws Exception {
     schemaString = "schema-spatial.xml";
     configString = "solrconfig-basic.xml";
 
+    //nocommit are we sure we do this in Gradle

Review comment:
       We do. See defaults-tests.gradle.




----------------------------------------------------------------
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