[
https://issues.apache.org/jira/browse/BEAM-8024?focusedWorklogId=492732&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-492732
]
ASF GitHub Bot logged work on BEAM-8024:
----------------------------------------
Author: ASF GitHub Bot
Created on: 29/Sep/20 23:47
Start Date: 29/Sep/20 23:47
Worklog Time Spent: 10m
Work Description: kennknowles commented on a change in pull request
#12970:
URL: https://github.com/apache/beam/pull/12970#discussion_r497145208
##########
File path: sdks/java/testing/jpms-tests/build.gradle
##########
@@ -31,13 +34,66 @@ enableJavaPerformanceTesting()
description = "Apache Beam :: SDKs :: Java :: Testing :: JPMS Tests"
ext.summary = "E2E test for Java 9 modules"
+/*
+ * List of runners to run post commit tests on.
+ */
+def postCommitRunnerClass = [
+ directRunner: "org.apache.beam.runners.direct.DirectRunner",
+ flinkRunner: "org.apache.beam.runners.flink.TestFlinkRunner",
+ dataflowRunner: "org.apache.beam.runners.dataflow.TestDataflowRunner",
+]
+for (String runner : postCommitRunnerClass.keySet()) {
+ configurations.create(runner + "PostCommit")
+}
+
dependencies {
compile project(path: ":sdks:java:core", configuration: "shadow")
- compile project(path: ":runners:direct-java")
compile project(path: ":sdks:java:extensions:google-cloud-platform-core")
testCompile library.java.junit
testCompile library.java.hamcrest_core
+
+ // Add dependencies for the PostCommit configurations
+ // For each runner a project level dependency on the test project.
+ for (String runner : postCommitRunnerClass.keySet()) {
+ delegate.add(runner + "PostCommit",
project(":sdks:java:testing:jpms-tests"))
+ delegate.add(runner + "PostCommit", project(path:
":sdks:java:testing:jpms-tests", configuration: "testRuntime"))
+ }
+ directRunnerPostCommit project(":runners:direct-java")
Review comment:
Especially considering this bit, the looping over configuration doesn't
really save you much. Could be simpler to just define 3 configs with very
straightforward control flow and data flow.
##########
File path: .test-infra/jenkins/job_PostCommit_Java_Jpms_Java11.groovy
##########
@@ -39,7 +39,7 @@
PostcommitJobBuilder.postCommitJob('beam_PostCommit_Java_Jpms_Java11', 'Run Jpms
steps {
gradle {
rootBuildScriptDir(commonJobProperties.checkoutDir)
- tasks(':sdks:java:testing:jpms-tests:integrationTest')
+ tasks(':sdks:java:testing:jpms-tests:postCommit')
Review comment:
Noted below that coupling the various runners has some drawbacks. It is
some boilerplate but can we factor it out into a few jobs: direct (candidate
for precommit), flink (also candidate, but slower), dataflow (too slow for
precommit).
##########
File path: sdks/java/testing/jpms-tests/build.gradle
##########
@@ -31,13 +34,66 @@ enableJavaPerformanceTesting()
description = "Apache Beam :: SDKs :: Java :: Testing :: JPMS Tests"
ext.summary = "E2E test for Java 9 modules"
+/*
+ * List of runners to run post commit tests on.
+ */
+def postCommitRunnerClass = [
+ directRunner: "org.apache.beam.runners.direct.DirectRunner",
+ flinkRunner: "org.apache.beam.runners.flink.TestFlinkRunner",
+ dataflowRunner: "org.apache.beam.runners.dataflow.TestDataflowRunner",
+]
+for (String runner : postCommitRunnerClass.keySet()) {
Review comment:
Aesthetically, I dislike building code identifiers (or similar) by
string concat. Pragmatically it makes it hard to search for them. But looping
out these configs makes sense. What do you think about something like a map
from configuration name to runner class.
```
def postCommitConfigurations = {
"directRunnerPostCommit" : "org.apache.beam.runners.direct.DirectRunner",
"flinkRunnerPostCommit" : "org.apache.beam.runners.flink.TestFlinkRunner"
"dataflowRunnerPostCommit" :
"org.apache.beam.runners.dataflow.TestDataflowRunner"
}
```
##########
File path: sdks/java/testing/jpms-tests/build.gradle
##########
@@ -31,13 +34,66 @@ enableJavaPerformanceTesting()
description = "Apache Beam :: SDKs :: Java :: Testing :: JPMS Tests"
ext.summary = "E2E test for Java 9 modules"
+/*
+ * List of runners to run post commit tests on.
+ */
+def postCommitRunnerClass = [
+ directRunner: "org.apache.beam.runners.direct.DirectRunner",
+ flinkRunner: "org.apache.beam.runners.flink.TestFlinkRunner",
+ dataflowRunner: "org.apache.beam.runners.dataflow.TestDataflowRunner",
+]
+for (String runner : postCommitRunnerClass.keySet()) {
+ configurations.create(runner + "PostCommit")
+}
+
dependencies {
compile project(path: ":sdks:java:core", configuration: "shadow")
- compile project(path: ":runners:direct-java")
compile project(path: ":sdks:java:extensions:google-cloud-platform-core")
testCompile library.java.junit
testCompile library.java.hamcrest_core
+
+ // Add dependencies for the PostCommit configurations
+ // For each runner a project level dependency on the test project.
+ for (String runner : postCommitRunnerClass.keySet()) {
+ delegate.add(runner + "PostCommit",
project(":sdks:java:testing:jpms-tests"))
+ delegate.add(runner + "PostCommit", project(path:
":sdks:java:testing:jpms-tests", configuration: "testRuntime"))
+ }
+ directRunnerPostCommit project(":runners:direct-java")
+ flinkRunnerPostCommit project(":runners:flink:1.10")
+ dataflowRunnerPostCommit project(":runners:google-cloud-dataflow-java")
+}
+
+/*
+ * Create a ${runner}PostCommit task for each runner which runs a set
+ * of integration tests for WordCount and WindowedWordCount.
+ */
+def gcpProject = project.findProperty('gcpProject') ?: 'apache-beam-testing'
Review comment:
putting a pin in the fact that this is largely independent of your
change and applies cross module (no action required)
##########
File path: sdks/java/testing/jpms-tests/build.gradle
##########
@@ -31,13 +34,66 @@ enableJavaPerformanceTesting()
description = "Apache Beam :: SDKs :: Java :: Testing :: JPMS Tests"
ext.summary = "E2E test for Java 9 modules"
+/*
+ * List of runners to run post commit tests on.
+ */
+def postCommitRunnerClass = [
+ directRunner: "org.apache.beam.runners.direct.DirectRunner",
+ flinkRunner: "org.apache.beam.runners.flink.TestFlinkRunner",
+ dataflowRunner: "org.apache.beam.runners.dataflow.TestDataflowRunner",
+]
+for (String runner : postCommitRunnerClass.keySet()) {
+ configurations.create(runner + "PostCommit")
+}
+
dependencies {
compile project(path: ":sdks:java:core", configuration: "shadow")
- compile project(path: ":runners:direct-java")
compile project(path: ":sdks:java:extensions:google-cloud-platform-core")
testCompile library.java.junit
testCompile library.java.hamcrest_core
+
+ // Add dependencies for the PostCommit configurations
+ // For each runner a project level dependency on the test project.
+ for (String runner : postCommitRunnerClass.keySet()) {
+ delegate.add(runner + "PostCommit",
project(":sdks:java:testing:jpms-tests"))
+ delegate.add(runner + "PostCommit", project(path:
":sdks:java:testing:jpms-tests", configuration: "testRuntime"))
+ }
+ directRunnerPostCommit project(":runners:direct-java")
+ flinkRunnerPostCommit project(":runners:flink:1.10")
+ dataflowRunnerPostCommit project(":runners:google-cloud-dataflow-java")
+}
+
+/*
+ * Create a ${runner}PostCommit task for each runner which runs a set
+ * of integration tests for WordCount and WindowedWordCount.
+ */
+def gcpProject = project.findProperty('gcpProject') ?: 'apache-beam-testing'
+def gcpRegion = project.findProperty('gcpRegion') ?: 'us-central1'
+def gcsTempRoot = project.findProperty('gcsTempRoot') ?:
'gs://temp-storage-for-end-to-end-tests/'
+
+for (String runner : postCommitRunnerClass.keySet()) {
+ tasks.create(name: runner + "PostCommit", type: Test) {
+ def postCommitBeamTestPipelineOptions = [
+ "--project=${gcpProject}",
+ "--tempRoot=${gcsTempRoot}",
+ "--runner=" + postCommitRunnerClass[runner],
+ ]
+ if ("dataflowRunner".equals(runner)) {
+ postCommitBeamTestPipelineOptions.add("--region=${gcpRegion}")
+ }
+ classpath = configurations."${runner}PostCommit"
+ include "**/*IT.class"
+ maxParallelForks 4
+ systemProperty "beamTestPipelineOptions",
JsonOutput.toJson(postCommitBeamTestPipelineOptions)
+ }
+}
+
+/* Define a common postcommit task which depends on all the individual
postcommits. */
Review comment:
This is a laudable instinct, but I actually don't favor it. Reasons:
1. Prefer to separate test suites from when they may run
2. Runners often have fairly different infrastructures to spin up and
potentially blow memory if accidentally run together
3. We want separate signals about what is unhealthy.
Regarding naming of a test suite "postcommit" I want to acknowledge that you
are noticing how things are currently organized and following those
conventions. So that's sensible. But it is a convention I think we have to move
away from. Gradle can be run from lots of places and the notion of precommit vs
postcommit is inessential.
----------------------------------------------------------------
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:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 492732)
Time Spent: 7h (was: 6h 50m)
> Project importing Beam that uses Java 11 and JPMS cannot compile
> ----------------------------------------------------------------
>
> Key: BEAM-8024
> URL: https://issues.apache.org/jira/browse/BEAM-8024
> Project: Beam
> Issue Type: Sub-task
> Components: build-system
> Reporter: Lukasz Gajowy
> Assignee: Kiley Sok
> Priority: P3
> Labels: java11
> Time Spent: 7h
> Remaining Estimate: 0h
>
> This was suggested on the devlist in a [thread about Java 11
> compatibility|https://lists.apache.org/thread.html/065e7cb9f25a4312ce5f0045ee2946c1d93c04615e37351f1e25a9b8@%3Cdev.beam.apache.org%3E].
>
> I confirmed it in my sandbox project. Source code:
> [https://github.com/lgajowy/beamJava11Compatibility]
> When building this sample project and importing 3 beam dependencies:
> {code:java}
> dependencies {
> compile group: 'org.apache.beam', name: 'beam-sdks-java-core', version:
> '2.14.0'
> compile group: 'org.apache.beam', name: 'beam-runners-direct-java',
> version: '2.14.0'
> compile group: 'org.apache.beam', name:
> 'beam-sdks-java-extensions-google-cloud-platform-core', version: '2.14.0'
> } {code}
>
> It is impossible to compile the project due to split-package problems:
> {code:java}
> error: the unnamed module reads package com.google.common.flogger from both
> flogger and google.extensions
> error: module beam.sdks.java.extensions.google.cloud.platform.core reads
> package com.google.common.flogger from both flogger and google.extensions
> error: module beam.model.pipeline reads package com.google.common.flogger
> from both flogger and google.extensions
> error: module beam.model.job.management reads package
> com.google.common.flogger from both flogger and google.extensions
> error: module beam.vendor.guava.20.0 reads package com.google.common.flogger
> from both flogger and google.extensions
> error: module com.fasterxml.jackson.databind reads package
> com.google.common.flogger from both flogger and google.extensions
> error: module google.auth.library.oauth2.http reads package
> com.google.common.flogger from both flogger and google.extensions
> error: module google.api.services.cloudresourcemanager.v1.rev20181015 reads
> package com.google.common.flogger from both flogger and google.extensions
> error: module gcsio reads package com.google.common.flogger from both flogger
> and google.extensions
> error: module util reads package com.google.common.flogger from both flogger
> and google.extensions
> ...{code}
>
> as stated in the devlist thread, more dependencies can cause problems.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
