-
Notifications
You must be signed in to change notification settings - Fork 63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Java module descriptor configuration #138
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,6 +57,7 @@ plugins { | |
id("com.asarkar.gradle.build-time-tracker") version "3.0.1" | ||
id("org.jetbrains.dokka") version "1.6.21" | ||
id("ru.vyarus.use-python") version "2.3.0" | ||
id("org.moditect.gradleplugin") version "1.0.0-rc3" | ||
id("com.github.johnrengelman.shadow") version "7.1.2" | ||
id("io.github.gradle-nexus.publish-plugin") version "1.1.0" | ||
`maven-publish` | ||
|
@@ -98,6 +99,27 @@ tasks.test { | |
maxParallelForks = 1 | ||
} | ||
|
||
// Suppress warnings about incubating test suites feature | ||
@Suppress("UnstableApiUsage") | ||
testing { | ||
suites { | ||
// Separate test suite for module testing | ||
val testModular by registering(JvmTestSuite::class) { | ||
dependencies { | ||
implementation(project) | ||
} | ||
} | ||
} | ||
} | ||
|
||
// Module tests require Java 9 or newer to compile and execute | ||
if (JavaVersion.current().isJava9Compatible) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably not ideal that this test won't be executed if a user builds the project with JDK 8. It seems to be possible to use a Gradle Toolchain for compilation and execution, but configuration would be a bit cumbersome at the moment because the test suite does not seem to support that directly yet for compilation (would have to specify it separately for the compile task). Should I try to implement that anyway? Alternatively in the future it could be considered to specify a toolchain for the complete build to make it more reproducible, regardless of which JDK version a user has installed. But that would require changes to the GitHub workflow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No, you don't have to do that. I'm happy with the current solution already. |
||
tasks.check { | ||
@Suppress("UnstableApiUsage") | ||
dependsOn(testing.suites.named("testModular")) | ||
} | ||
} | ||
|
||
tasks.jacocoTestReport { | ||
dependsOn("test") | ||
reports { | ||
|
@@ -250,6 +272,22 @@ tasks.register<PythonTask>("writeAccuracyTable") { | |
command = "src/python-scripts/write_accuracy_table.py" | ||
} | ||
|
||
tasks.addMainModuleInfo { | ||
version = project.version | ||
// Create Multi-Release JAR with Java 9 as lowest version | ||
jvmVersion.set("9") | ||
// Overwrite the output JAR file (if any) from a previous Gradle execution | ||
overwriteExistingFiles.set(true) | ||
module { | ||
moduleInfoFile = File("$projectDir/src/main/java-9/module-info.java") | ||
} | ||
} | ||
// Workaround to avoid circular dependencies between tasks, see https://github.com/moditect/moditect-gradle-plugin/issues/14 | ||
project.afterEvaluate { | ||
val compileJavaTask = tasks.compileJava.get() | ||
compileJavaTask.setDependsOn(compileJavaTask.dependsOn - tasks.addDependenciesModuleInfo.get()) | ||
} | ||
|
||
tasks.withType<DokkaTask>().configureEach { | ||
dokkaSourceSets.configureEach { | ||
jdkVersion.set(6) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
module com.github.pemistahl.lingua { | ||
exports com.github.pemistahl.lingua.api; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might also need to export the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, the |
||
|
||
requires kotlin.stdlib; | ||
|
||
requires it.unimi.dsi.fastutil; | ||
requires okio; | ||
|
||
// Moshi accesses JSON serializer using reflection; must open the package | ||
// TODO: Once new Moshi version has module names, change it to `opens ... to com.squareup.moshi` | ||
// and comment in the `requires` declarations below | ||
opens com.github.pemistahl.lingua.internal; | ||
// requires com.squareup.moshi; | ||
// requires com.squareup.moshi.kotlin; | ||
Comment on lines
+13
to
+14
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moshi already has an |
||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
// Open complete module for reflection to allow JUnit to access the packages | ||
open module test { | ||
requires com.github.pemistahl.lingua; | ||
|
||
requires org.junit.jupiter.api; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package test; | ||
|
||
import com.github.pemistahl.lingua.api.*; | ||
import org.junit.jupiter.api.Test; | ||
|
||
import static com.github.pemistahl.lingua.api.Language.*; | ||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
||
/** | ||
* Tests basic Lingua functionality. The main purpose of this test is to verify that the | ||
* packages can be accessed from a different module and that Lingua specifies all required | ||
* modules and can be used successfully. | ||
*/ | ||
class LinguaTest { | ||
@Test | ||
void test() { | ||
LanguageDetector detector = LanguageDetectorBuilder.fromLanguages(ENGLISH, FRENCH, GERMAN, SPANISH).build(); | ||
Language detectedLanguage = detector.detectLanguageOf("languages are awesome"); | ||
assertEquals(ENGLISH, detectedLanguage); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once the test suite feature is stable it might be reasonable to convert the other test definitions to test suites as well.