Skip to content
This repository was archived by the owner on Sep 16, 2021. It is now read-only.

mvn clean test failing #262

Closed
gregw opened this issue May 25, 2016 · 15 comments
Closed

mvn clean test failing #262

gregw opened this issue May 25, 2016 · 15 comments
Assignees
@joakime @jmcc0nn3ll
Labels
bug
Milestone
async-support

Comments

@gregw
Copy link
Contributor

gregw commented May 25, 2016

In the async-support branch a mvn clean test fails, but mvn clean install passed
@gregw gregw added the bug label May 25, 2016
@aozarov aozarov added this to the Beta milestone Jun 1, 2016
@gregw
Copy link
Contributor Author

gregw commented Jun 1, 2016

@joakime you might want to have a look at this also

@joakime
Copy link

joakime commented Jun 1, 2016 •
edited
Loading

Some problems identified in the build that need to be fixed:

  • The docker-maven-plugin isn't run in the 'mvn clean test' build, making the reference later in the 'mvn clean test' invalid
  • The manual use of maven-resources-plugin needs to be fixed to use the //build/resources instead (the current build puts maven-resources-plugin in the wrong places in the build lifecycle)
  • The use of maven-dependency-plugin:copy-dependencies must be after the package phase in the build (/jetty9-base/ violates this), and it currently isn't being used that way.

@joakime
Copy link

joakime commented Jun 3, 2016 •
edited
Loading

After a review, the tests that use docker are not going to work on the test phase, but are more appropriate at the integration-test phase.

See: http://maven.apache.org/ref/3.3.1/maven-core/lifecycles.html

All of the docker related tests need to go through the package phase properly for it to be sane. Putting the docker related testing in integration-test is the most sane way to accomplish this.

Working on a good PR for this approach.

@joakime
Copy link

joakime commented Jun 3, 2016

In fact ....

Use mvn clean integration-test now, it works.

I will still submit a PR for fixing up the <build><resources> issues discovered in this analysis.

@aozarov
Copy link
Contributor

aozarov commented Jun 3, 2016 •
edited
Loading

I think mvn clean test should work too, though I imagine, run only the tests that are not consider "integration test". I guess one way to separate them is by naming convention and using ITXXX for integration tests?

@joakime
Copy link

joakime commented Jun 3, 2016

All of the docker related creation / building will need to be the "line in the sand". if the module does anything with docker, it needs to be at the package phase as well. The regular tests cannot rely on Docker or anything docker related or created to function.

@aozarov
Copy link
Contributor

aozarov commented Jun 3, 2016

Yes, I understand that. Are you saying we don't have any unit-tests that don't depend on docker?

@joakime
Copy link

joakime commented Jun 3, 2016

The tests are being evaluated ATM, seeing where each would live. stay tuned.

@joakime
Copy link

joakime commented Jun 3, 2016

So far, everything in jetty9-compat-base would be an integration test.

This is because the dependency:copy (for jetty9-base, and testwebapp.war) cannot work in a reactor build that hasn't had the package phase executed for those projects (note: package is a phase after test). Making the requirements for the tests in jetty9-compat-base invalid for the test phase.

@joakime
Copy link

joakime commented Jun 3, 2016

Here's an interesting question (coming from an emeritus apache maven pmc member no less!)...

Since so many other Google projects seems so enamored with gradle, why is this build not using gradle? (Or to put it a different way, is there an expectation for this project to be gradle-ized in the future to conform to some Google project policy?)

joakime added a commit to jetty-project/appengine-java-vm-runtime that referenced this issue Jun 3, 2016
@joakime
Fixes GoogleCloudPlatform#262 - make 'mvn clean test' work
b3ea313 
+ Reworking jetty9-base and jetty9-compat-base to utilize
  integration-test phases properly for the docker, dependency:copy, and
  dependency:copy-dependencies limitations that crop up when you want to
  utilize reactor projects that haven't been packaged, but are required
  to be packaged in order to run testing, assembly, or the dependency
  plugin
@aozarov
Copy link
Contributor

aozarov commented Jun 4, 2016

I don't think we should ditch maven anytime soon. There are many users using it and many of our tools are built around it. However +1 for Making our build support both maven and gradle.

I don't think there is any strict policy about it so far. Some google projects are using only maven (guava, guice,...) and others (not as many) use gradle (such as grpc-java and looking at their gradle config file makes my head spin).

aozarov pushed a commit that referenced this issue Jun 4, 2016
@joakime @aozarov
Fixes #262 - make 'mvn clean test' work (#275)
ddb7f50 
Reworking jetty9-base and jetty9-compat-base to utilize
integration-test phases properly for the docker, dependency:copy, and
dependency:copy-dependencies limitations that crop up when you want to
utilize reactor projects that haven't been packaged, but are required
to be packaged in order to run testing, assembly, or the dependency
plugin
@aozarov
Copy link
Contributor

aozarov commented Jun 7, 2016

Fixed by #275

@aozarov aozarov closed this as completed Jun 7, 2016
@aozarov aozarov reopened this Jun 8, 2016
@aozarov
Copy link
Contributor

aozarov commented Jun 8, 2016

@joakime Though the build completes fine are you sure the it/... tests are actually running upon install, verify or integration-test?

@aozarov aozarov modified the milestones: async-support, Beta Jun 8, 2016
@joakime joakime mentioned this issue Jun 8, 2016
Closed
@joakime
Copy link

joakime commented Jun 8, 2016

Will manually declare the failsafe plugin (configuration & version) in the top level pom.
Stay tuned.

joakime added a commit to jetty-project/appengine-java-vm-runtime that referenced this issue Jun 9, 2016
@joakime
Fixes GoogleCloudPlatform#262 - Integration testing more stable in ma…
52ffd07 
…ven.

+ Forces maven-failsafe-plugin in parent (with version)
+ Moves jetty9-compact-base tests back into src/test/java
+ Renames integration tests from *Test.java to *IT.java to
  better conform to new recommendations from maven-failsafe-plugin
  (convention over configuration!)
joakime added a commit to jetty-project/appengine-java-vm-runtime that referenced this issue Jun 9, 2016
@joakime
Issue GoogleCloudPlatform#262 - src/test/java passed through google-j…
02edd9a 
…ava-format-1.0-all-deps.jar
joakime added a commit to jetty-project/appengine-java-vm-runtime that referenced this issue Jun 9, 2016
@joakime
Issue GoogleCloudPlatform#262 - Checkstyle updates
4405b39 
+ Fixed import order issues
+ Added checkstyle/checkstyle-suppressions.xml
+ Updated root pom.xml to use new suppressions techniques in a way
  that's friendly to a maven reactor build
aozarov pushed a commit that referenced this issue Jun 9, 2016
@joakime @aozarov
Fixes #262 - Integration testing more stable in maven. (#283)
b9e3865 
* Fixes #262 - Integration testing more stable in maven.
* Forces maven-failsafe-plugin in parent (with version)
* Renames integration tests from *Test.java to *IT.java to
  better conform to new recommendations from maven-failsafe-plugin
  (convention over configuration!)
* Issue #262 - Checkstyle updates
* Added checkstyle/checkstyle-suppressions.xml
* Updated root pom.xml to use new suppression techniques in a way
  that's friendly to a maven reactor build
@joakime
Copy link

joakime commented Jun 10, 2016

Closing, as PR #283 is applied

@joakime joakime closed this as completed Jun 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@joakime joakime

@jmcc0nn3ll jmcc0nn3ll

Labels
bug
Projects
None yet
Milestone
async-support
Development

No branches or pull requests
4 participants
@gregw @joakime @jmcc0nn3ll @aozarov