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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
