LUCENE-10328: Module path for compiling and running tests is wrong (a…

…pache#571)

gradle/documentation/render-javadoc.gradle

@@ -59,7 +59,7 @@ allprojects {
       outputDir = project.javadoc.destinationDir
     }
 
-    if (project.path == ':lucene:luke' || project.path.endsWith(".tests")) {
+    if (project.path == ':lucene:luke' || !(project in rootProject.ext.mavenProjects)) {
       // These projects are not part of the public API so we don't render their javadocs
       // as part of the site's creation. A side-effect of this is that javadocs would not
       // be linted for these projects. To avoid this, we connect the regular javadoc task
@@ -167,26 +167,6 @@ configure(project(":lucene:test-framework")) {
   project.tasks.withType(RenderJavadocTask) {
     // TODO: fix missing javadocs
     javadocMissingLevel = "class"
-    // TODO: clean up split packages
-    javadocMissingIgnore = [
-        "org.apache.lucene.analysis",
-        "org.apache.lucene.analysis.standard",
-        "org.apache.lucene.codecs",
-        "org.apache.lucene.codecs.blockterms",
-        "org.apache.lucene.codecs.bloom",
-        "org.apache.lucene.codecs.compressing",
-        "org.apache.lucene.codecs.uniformsplit",
-        "org.apache.lucene.codecs.uniformsplit.sharedterms",
-        "org.apache.lucene.geo",
-        "org.apache.lucene.index",
-        "org.apache.lucene.search",
-        "org.apache.lucene.search.similarities",
-        "org.apache.lucene.search.spans",
-        "org.apache.lucene.store",
-        "org.apache.lucene.util",
-        "org.apache.lucene.util.automaton",
-        "org.apache.lucene.util.fst"
-    ]
   }
 }
 
@@ -197,6 +177,13 @@ configure(project(":lucene:sandbox")) {
   }
 }
 
+configure(project(":lucene:spatial-test-fixtures")) {
+  project.tasks.withType(RenderJavadocTask) {
+    // TODO: fix missing javadocs
+    javadocMissingLevel = "class"
+  }
+}
+
 configure(project(":lucene:misc")) {
   project.tasks.withType(RenderJavadocTask) {
     // TODO: fix missing javadocs

gradle/maven/publications.gradle

@@ -38,7 +38,9 @@ configure(rootProject) {
           // Exclude the parent container project for analysis modules (no artifacts).
           ":lucene:analysis",
           // Exclude the native module.
-          ":lucene:misc:native"
+          ":lucene:misc:native",
+          // Exclude test fixtures.
+          ":lucene:spatial-test-fixtures"
       ]
 
       // Exclude all subprojects that are modular test projects and those explicitly

gradle/validation/check-environment.gradle

@@ -33,14 +33,20 @@ configure(rootProject) {
   def currentJavaVersion = JavaVersion.current()
   if (currentJavaVersion < minJavaVersion) {
     throw new GradleException("At least Java ${minJavaVersion} is required, you are running Java ${currentJavaVersion} "
-      + "[${System.getProperty('java.vm.name')} ${System.getProperty('java.vm.version')}]")
+        + "[${System.getProperty('java.vm.name')} ${System.getProperty('java.vm.version')}]")
   }
 
   // If we're regenerating the wrapper, skip the check.
   if (!gradle.startParameter.taskNames.contains("wrapper")) {
     def currentGradleVersion = GradleVersion.current()
     if (currentGradleVersion != GradleVersion.version(expectedGradleVersion)) {
-      throw new GradleException("Gradle ${expectedGradleVersion} is required (hint: use the gradlew script): this gradle is ${currentGradleVersion}")
+      if (currentGradleVersion.baseVersion == GradleVersion.version(expectedGradleVersion).baseVersion) {
+        logger.warn("Gradle ${expectedGradleVersion} is required but base version of this gradle matches, proceeding (" +
+            "this gradle is ${currentGradleVersion})")
+      } else {
+        throw new GradleException("Gradle ${expectedGradleVersion} is required (hint: use the gradlew script): " +
+            "this gradle is ${currentGradleVersion}")
+      }
     }
   }
 }

gradle/validation/ecj-lint.gradle

@@ -74,42 +74,36 @@ allprojects {
         args += [ "-properties", file("${resources}/ecj.javadocs.prefs").absolutePath ]
 
         // We depend on modular paths.
-        def modularPaths = sourceSet.modularPaths
-        dependsOn modularPaths.compileModulePathConfiguration
+        def modularPaths = sourceSet.modularPathsForEcj
+        dependsOn modularPaths
 
-        // Place input files in an external file to dodge command line argument
-        // limits. We could pass a directory but ecj seems to be buggy: when it
-        // encounters a module-info.java file it no longer compiles other source files.
-        def inputsFile = file("${tmpDst}/ecj-inputs.txt")
+        // Add modular dependencies and their transitive dependencies to module path.
+        task.argumentProviders.add(modularPaths.compilationArguments)
 
+        // Add classpath, if needed.
         task.argumentProviders.add((CommandLineArgumentProvider) {
-          // Add modular dependencies and their transitive dependencies to module path.
-          def modularPathFiles = modularPaths.compileModulePathConfiguration.files
-          def extraArgs = []
-          if (!modularPathFiles.isEmpty()) {
-            if (!modularPaths.hasModuleDescriptor()) {
-              // We're compiling a non-module so we'll bring everything on module path in
-              // otherwise things wouldn't be part of the resolved module graph.
-              extraArgs += ["--add-modules", "ALL-MODULE-PATH"]
-            }
-
-            extraArgs += ["--module-path", modularPathFiles.join(File.pathSeparator)]
-          }
-
           // Add classpath locations in a lazy provider (can't resolve the
           // configuration at evaluation time). Filter out non-existing entries
           // (output folders for non-existing input source dirs like resources).
-          def cpath = sourceSet.compileClasspath.filter { p -> p.exists() }
-          cpath = cpath - modularPathFiles
+          FileCollection cpath = modularPaths.compilationClasspath.filter { p -> p.exists() }
           if (!cpath.isEmpty()) {
-            extraArgs += ["-classpath", cpath.join(File.pathSeparator)]
+            return ["-classpath", cpath.join(File.pathSeparator)]
+          } else {
+            return []
           }
+        })
 
-          extraArgs += ["@" + inputsFile.absolutePath]
-          return extraArgs
+        // Place input files in an external file to dodge command line argument
+        // limits. We could pass a directory but ecj seems to be buggy: when it
+        // encounters a module-info.java file it no longer compiles other source files.
+        def inputsFile = file("${tmpDst}/ecj-inputs.txt")
+        task.argumentProviders.add((CommandLineArgumentProvider) {
+          return ["@" + inputsFile.absolutePath]
         })
 
         doFirst {
+          modularPaths.logCompilationPaths(logger)
+
           tmpDst.mkdirs()
 
           // escape filename accoring to ECJ's rules:
@@ -118,7 +112,12 @@ allprojects {
           def escapeFileName = { String s -> s.replaceAll(/ +/, /"$0"/) }
           inputsFile.setText(
             srcDirs.collectMany { dir ->
-              project.fileTree(dir: dir, include: "**/*.java" ).files
+              project.fileTree(
+                  dir: dir,
+                  include: "**/*.java",
+                  // Exclude the benchmark class with dependencies on nekohtml, which causes module-classpath conflicts and breaks ecj.
+                  exclude: "**/DemoHTMLParser.java"
+              ).files
             }
             // Try to sort all input files; a side-effect of this should be that module-info.java
             // is placed first on the list, which works around ECJ bug:

gradle/validation/validate-source-patterns.gradle

@@ -150,12 +150,11 @@ class ValidateSourcePatternsTask extends DefaultTask {
       (~$/\n\s*var\s+.*=.*<>.*/$) : 'Diamond operators should not be used with var'
     ]
 
-    def found = 0;
     def violations = new TreeSet();
     def reportViolation = { f, name ->
-      logger.error('{}: {}', name, f);
-      violations.add(name);
-      found++;
+      String msg = String.format(Locale.ROOT, "%s: %s", f, name)
+      logger.error(msg)
+      violations.add(msg)
     }
 
     def javadocsPattern = ~$/(?sm)^\Q/**\E(.*?)\Q*/\E/$;
@@ -254,9 +253,10 @@ class ValidateSourcePatternsTask extends DefaultTask {
     }
     progress.completed()
 
-    if (found) {
-      throw new GradleException(String.format(Locale.ENGLISH, 'Found %d violations in source files (%s).',
-        found, violations.join(', ')));
+    if (!violations.isEmpty()) {
+      throw new GradleException(String.format(Locale.ENGLISH, 'Found %d source violation(s):\n  %s',
+        violations.size(),
+        violations.join('\n  ')))
     }
   }
 }

lucene/analysis/common/build.gradle

@@ -21,7 +21,7 @@ description = 'Analyzers for indexing content in different languages and domains
 
 dependencies {
   moduleApi project(':lucene:core')
-  testImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:test-framework')
 }
 
 // Fetch the data and enable regression tests against woorm/ libreoffice dictionaries.

lucene/analysis/icu/build.gradle

@@ -25,5 +25,5 @@ dependencies {
 
   moduleApi 'com.ibm.icu:icu4j'
 
-  testImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:test-framework')
 }

lucene/analysis/kuromoji/build.gradle

@@ -23,5 +23,5 @@ dependencies {
   moduleApi project(':lucene:core')
   moduleApi project(':lucene:analysis:common')
 
-  testImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:test-framework')
 }

lucene/analysis/morfologik.tests/build.gradle

@@ -21,8 +21,5 @@ description = 'Module tests for :lucene:analysis:morfologik'
 
 dependencies {
   moduleTestImplementation project(':lucene:analysis:morfologik')
-  moduleTestImplementation("junit:junit", {
-    exclude group: "org.hamcrest"
-  })
-  moduleTestImplementation "org.hamcrest:hamcrest"
+  moduleTestImplementation project(':lucene:test-framework')
 }

lucene/analysis/morfologik.tests/src/test/module-info.java

@@ -21,7 +21,7 @@
   requires org.apache.lucene.core;
   requires org.apache.lucene.analysis.common;
   requires org.apache.lucene.analysis.morfologik;
-  requires junit;
+  requires org.apache.lucene.test_framework;
 
   exports org.apache.lucene.analysis.morfologik.tests;
 }

lucene/analysis/morfologik.tests/src/test/org/apache/lucene/analysis/morfologik/tests/TestMorfologikAnalyzer.java

@@ -19,28 +19,24 @@
 import org.apache.lucene.analysis.morfologik.MorfologikAnalyzer;
 import org.apache.lucene.analysis.uk.UkrainianMorfologikAnalyzer;
 import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.tests.util.LuceneTestCase;
 import org.junit.Assert;
-import org.junit.Test;
 
-public class TestMorfologikAnalyzer {
-  @Test
+public class TestMorfologikAnalyzer extends LuceneTestCase {
   public void testMorfologikAnalyzerLoads() {
     var analyzer = new MorfologikAnalyzer();
     Assert.assertNotNull(analyzer);
   }
 
-  @Test
   public void testUkrainianMorfologikAnalyzerLoads() {
     var analyzer = new UkrainianMorfologikAnalyzer();
     Assert.assertNotNull(analyzer);
   }
 
-  @Test
   public void testWeAreModule() {
     Assert.assertTrue(this.getClass().getModule().isNamed());
   }
 
-  @Test
   public void testLuceneIsAModule() {
     Assert.assertTrue(IndexWriter.class.getModule().isNamed());
   }

lucene/analysis/morfologik/build.gradle

@@ -27,5 +27,5 @@ dependencies {
   moduleImplementation 'org.carrot2:morfologik-polish'
   moduleImplementation 'ua.net.nlp:morfologik-ukrainian-search'
 
-  testImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:test-framework')
 }

lucene/analysis/nori/build.gradle

@@ -22,7 +22,7 @@ description = 'Korean Morphological Analyzer'
 dependencies {
   moduleApi project(':lucene:core')
   moduleApi project(':lucene:analysis:common')
-  
-  testImplementation project(':lucene:test-framework')
+
+  moduleTestImplementation project(':lucene:test-framework')
 }
 

lucene/analysis/opennlp/build.gradle

@@ -24,5 +24,5 @@ dependencies {
   moduleApi project(':lucene:analysis:common')
   moduleApi 'org.apache.opennlp:opennlp-tools'
 
-  testImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:test-framework')
 }

lucene/analysis/phonetic/build.gradle

@@ -25,6 +25,6 @@ dependencies {
 
   moduleApi 'commons-codec:commons-codec'
 
-  testImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:test-framework')
 } 
 

lucene/analysis/smartcn/build.gradle

@@ -23,5 +23,5 @@ dependencies {
   moduleApi project(':lucene:core')
   moduleApi project(':lucene:analysis:common')
 
-  testImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:test-framework')
 } 

lucene/analysis/stempel/build.gradle

@@ -23,5 +23,5 @@ dependencies {
   moduleApi project(':lucene:core')
   moduleApi project(':lucene:analysis:common')
 
-  testImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:test-framework')
 }

lucene/backward-codecs/build.gradle

@@ -22,5 +22,5 @@ description = 'Codecs for older versions of Lucene'
 
 dependencies {
   moduleApi project(':lucene:core')
-  testImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:test-framework')
 }

lucene/benchmark/build.gradle

@@ -36,11 +36,18 @@ dependencies {
   moduleImplementation "org.locationtech.spatial4j:spatial4j"
   moduleImplementation ("net.sourceforge.nekohtml:nekohtml", {
     exclude module: "xml-apis"
+    // LUCENE-10337: Exclude xercesImpl from module path because it has split packages with the JDK (!)
+    exclude module: "xercesImpl"
+  })
+
+  // LUCENE-10337: Include xercesImpl on regular classpath where it won't cause conflicts.
+  implementation ("xerces:xercesImpl", {
+    exclude module: "xml-apis"
   })
 
   moduleRuntimeOnly project(':lucene:analysis:icu')
 
-  testImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:test-framework')
 }
 
 // We add 'conf' to resources because we validate *.alg script correctness in one of the tests.

lucene/classification/build.gradle

@@ -25,7 +25,7 @@ dependencies {
   moduleImplementation project(':lucene:queries')
   moduleImplementation project(':lucene:grouping')
 
-  testImplementation project(':lucene:test-framework')
-  testImplementation project(':lucene:analysis:common')
-  testImplementation project(':lucene:codecs')
+  moduleTestImplementation project(':lucene:test-framework')
+  moduleTestImplementation project(':lucene:analysis:common')
+  moduleTestImplementation project(':lucene:codecs')
 }

lucene/codecs/build.gradle

@@ -21,5 +21,5 @@ description = 'Lucene codecs and postings formats'
 
 dependencies {
     moduleImplementation project(':lucene:core')
-    testImplementation project(':lucene:test-framework')
+    moduleTestImplementation project(':lucene:test-framework')
 }

lucene/codecs/src/test/org/apache/lucene/codecs/lucene90/MockTermStateFactory.java → lucene/codecs/src/test/org/apache/lucene/codecs/lucene90/tests/MockTermStateFactory.java

@@ -15,13 +15,14 @@
  * limitations under the License.
  */
 
-package org.apache.lucene.codecs.lucene90;
+package org.apache.lucene.codecs.lucene90.tests;
 
-/** Test utility class to create mock {@link Lucene90PostingsFormat.IntBlockTermState}. */
-public class MockTermStateFactory {
+import org.apache.lucene.codecs.lucene90.Lucene90PostingsFormat.IntBlockTermState;
 
-  /** Creates an empty {@link Lucene90PostingsFormat.IntBlockTermState}. */
-  public static Lucene90PostingsFormat.IntBlockTermState create() {
-    return new Lucene90PostingsFormat.IntBlockTermState();
+/** Test utility class to create mock {@link IntBlockTermState}. */
+public class MockTermStateFactory {
+  /** Creates an empty {@link IntBlockTermState}. */
+  public static IntBlockTermState create() {
+    return new IntBlockTermState();
   }
 }

lucene/codecs/src/test/org/apache/lucene/codecs/uniformsplit/TestBlockWriter.java

@@ -19,7 +19,7 @@
 
 import java.io.IOException;
 import java.util.Collections;
-import org.apache.lucene.codecs.lucene90.MockTermStateFactory;
+import org.apache.lucene.codecs.lucene90.tests.MockTermStateFactory;
 import org.apache.lucene.index.DocValuesType;
 import org.apache.lucene.index.FieldInfo;
 import org.apache.lucene.index.IndexOptions;

lucene/codecs/src/test/org/apache/lucene/codecs/uniformsplit/sharedterms/TestSTBlockReader.java

@@ -27,7 +27,7 @@
 import java.util.Set;
 import org.apache.lucene.codecs.BlockTermState;
 import org.apache.lucene.codecs.PostingsReaderBase;
-import org.apache.lucene.codecs.lucene90.MockTermStateFactory;
+import org.apache.lucene.codecs.lucene90.tests.MockTermStateFactory;
 import org.apache.lucene.codecs.uniformsplit.BlockHeader;
 import org.apache.lucene.codecs.uniformsplit.BlockLine;
 import org.apache.lucene.codecs.uniformsplit.FSTDictionary;