Skip to content
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

Added explicit decencies for surefile-junit4 in order to fix testing in offline mode. #3820

Merged
merged 19 commits into from
Oct 29, 2018

Conversation

silverdev
Copy link
Contributor

@silverdev silverdev commented Oct 15, 2018

Fixes #3819 (it's a good idea to open an issue first for context and/or discussion)

Sam Laane added 3 commits October 11, 2018 13:42
Without this dependency explicitly listed java mvn dependency:go-offline will not pull it in and mvn -o verify will not work (Unless the tests have been run in online mode. In this case the cached surefire junit can still be used.)
@silverdev silverdev requested a review from a team as a code owner October 15, 2018 23:10
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 15, 2018
Copy link
Contributor

@chingor13 chingor13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please pull these changes into the parent google-cloud-clients/pom.xml. If it's affecting these 3 apis, then it will need to be fixed in all of them.

<dependency>
<groupId>org.apache.maven.surefire</groupId>
<artifactId>surefire-junit4</artifactId>
<version>2.12.4</version>

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

Sam Laane and others added 4 commits October 17, 2018 16:44
…urefire version 2.17

[INFO] Building Google Cloud Java Compatibility Checker 0.67.1-alpha-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[WARNING] The POM for org.apache.maven.plugins:maven-surefire-plugin:jar:2.17 is missing, no dependency information available
[INFO] ------------------------------------------------------------------------
[INFO] Building Google Cloud API gRPC 0.32.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[INFO]                                                                         
[INFO] ------------------------------------------------------------------------
[INFO] Building proto-google-cloud-asset-v1beta1 0.32.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[WARNING] The POM for org.apache.maven.plugins:maven-surefire-plugin:jar:2.17 is missing, no dependency information available
[INFO]                                                                         
[INFO] ------------------------------------------------------------------------
[INFO] Skipping grpc-google-cloud-asset-v1beta1
[INFO] This project has been banned from the build due to previous failures.
[INFO] ------------------------------------------------------------------------
[INFO]                                                                         
[INFO] ------------------------------------------------------------------------
[INFO] Building proto-google-cloud-automl-v1beta1 0.32.1-SNAPSHOT
[INFO] ------------------------------------------------------------------------
[WARNING] The POM for org.apache.maven.plugins:maven-surefire-plugin:jar:2.17 is missing, no dependency information available
[INFO]                                                                         
[INFO] ------------------------------------------------------------------------
[INFO] Skipping grpc-google-cloud-automl-v1beta1
[INFO] This project has been banned from the build due to previous failures.
[INFO] ------------------------------------------------------------------------
@chingor13
Copy link
Contributor

chingor13 commented Oct 18, 2018

All the versions of surefire should be the same within google-cloud-java. The version that other projects use is irrelevant as it is a test dependency - only used for building and running the tests this package.

@JesseLovelace JesseLovelace added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 23, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 23, 2018
@JesseLovelace JesseLovelace added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 24, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 24, 2018
@JesseLovelace JesseLovelace added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 25, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 25, 2018
@JesseLovelace JesseLovelace added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 25, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 25, 2018
@JesseLovelace JesseLovelace added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 27, 2018
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Oct 27, 2018
@JesseLovelace JesseLovelace dismissed chingor13’s stale review October 29, 2018 16:30

Discussed offline--we'll open another issue to fix the explicit dependencies

@JesseLovelace JesseLovelace self-requested a review October 29, 2018 16:30
@JesseLovelace JesseLovelace merged commit 5e0bca4 into googleapis:master Oct 29, 2018
@codahale
Copy link

This change added org.apache.maven.surefire:surefire-junit4:2.19.1 as a compile dependency to most of the GCP SDKs, which makes it impossible to depend on those SDKs and run tests in Maven.

@silverdev
Copy link
Contributor Author

silverdev commented Nov 20, 2018 via email

@codahale
Copy link

The version of surefire-junit4 depended on directly via the SDK overrules the version required by the local Maven install and it results in the test runner crashing due to missing methods or other incompatibilities. You’re not seeing this problem because this change pins the version of surefire-junit4 to the specific version your local Maven install requires. My environment is different—a newer version of Surefire—and this breaks it.

Maven dependencies have scopes—compile, provided, runtime, test, etc.—explicitly to limit transitivity of dependencies. This change adds the dependencies with the default scope—compile—and so every single project which depends on the SDK now also depends on a specific version of a specific Maven plugin. Not just in their build pipeline, but also in any JARs or WARs they produce. This is obviously not a good solution because nothing in the runtime code of these libraries even knows about Maven or JUnit, let alone depends on the code within.

Now, as to the problem: dependency:go-offline will pull in plugins in addition to dependencies. You do not need to make your project depend on all the dependencies of Maven itself to get offline builds to work. You just need to list dependencies which are discovered at runtime—like surefire-junit4—explicit dependencies of maven-surefire-plugin:

<build>
  <plugins>
    <plugin>
      <artifactId>maven-surefire-plugin</artifactId>
      <version>2.22.1</version>
      <dependencies>
        <dependency>
          <groupId>org.apache.maven.surefire</groupId>
          <artifactId>2.22.1</artifactId>
          <version></version>
        </dependency>
      </dependencies>
    </plugin>
  </plugins>
</build>

@silverdev
Copy link
Contributor Author

silverdev commented Nov 20, 2018 via email

@codahale
Copy link

That should work, yes.

indrekpr pushed a commit to sympower/slf4j that referenced this pull request Mar 19, 2019
…pache.maven.surefire:surefire-junit4:jar:2.20.1` and `dummy:dummy:jar:1.0` dependencies while offline

Apparently the `surefire-junit4` dependency needs to be listed explicitly as the plugin's dependency, so that `mvn dependency:go-offline` could see it needs to be downloaded too.

There was a nice discussion over here:
  googleapis/google-cloud-java#3819
  googleapis/google-cloud-java#3820
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants