-
Notifications
You must be signed in to change notification settings - Fork 560
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
[Backports stable/1.0] Separate unit and integration tests during build process #7334
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Splits unit and integration tests by which Maven plugin is used to run them. Integration tests are handled by Failsafe, and unit tests by Surefire. Introduces two new variables to allow exclusions of each type: - skipUTs: skips unit tests by disabling the execution of Surefire - skipITs: skips integration tests by disabling the execution of Failsafe - skipTests: using skipTests will still skip all tests, as before. Moves the update tests under the qa module to allow them to share the same testing configuration as the integration test module, and renames them from zeebe-update-tests to zeebe-qa-update-tests to align with the other modules. (cherry picked from commit 1a46f5a)
Moves a previously "unit" test which was really an integration test to be used by Failsafe. (cherry picked from commit 27d69e4)
Removes test markers and related profiles about UnstableCI and UnstableTest. We do want to run unstable tests as we want to fix them as soon as possible, therefore these profiles should not be (and were not) used. (cherry picked from commit 43f13fb)
Adds a new property called skipChecks, which disables almost all checks during build time. This is useful when running the verify lifecycle again for integration tests, which will run the checks again unnecessarily. The approach taken is a bit different: we add a new property which acts as a default value for pre-defined properties, e.g. license.skip, checkstyle.skip, etc. That way you can use the new property disable all checks, or use the existing properties to disable a single check. When running tests in CI, skip the checks as we already run them during the install phase. (cherry picked from commit 73900a5)
Only execute the test phase when running Java 8 specific tests. The whole verify phase is run when building in a separate stage, so there's no need to repeat this. (cherry picked from commit 31ab283)
Use package goal when preparing the IT agent instead of simply test-compile. This is because integration tests rely on the fact that the application is packaged, not just that the code is compiled. (cherry picked from commit 801a847)
Replaces usage of dependency-plugin to unpack the sibling dependency and simply access the protocol file directly. The main advantage here is that you don't need to package/deploy the sibling artifact before building this module anymore, you can simply run it and access the file directly. (cherry picked from commit 67d3ebc)
Remove integration of dind sibling for the maven unit test container, and scale down the resources of the dind sibling. It's still present in the distribution template because the Go integration tests rely on it, but we still want to make sure no unit tests on the Java side will work with it. Plus, it frees up more resources for us to parallelize our unit tests. (cherry picked from commit b793196)
Even though we skip the revapi during integration testing, we still need to ensure the org.revapi:revapi-java dependency (which the plugin requires) is present offline, as otherwise the plugin will fail to load and the execution is not skipped but rather failed. Additionally, since we now enforce it so it's downloaded, we need to make sure it's ignored even though it's unused. (cherry picked from commit bde52bd)
Specifies the complete parent path, including the pom.xml filename. While it's not strictly necessary, the Maven documentation uses this format, and the XSD for 4.0 validates the path including the filename, marking the path as invalid without the filename. (cherry picked from commit d61619b)
korthout
approved these changes
Jun 21, 2021
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.
LGTM 👍
- I don't mind the added test as part of this PR (even as part of that first commit), it fits nicely there
- I did not see anything standing out in the last commit
Updates the zeebe-test-container version so it's compatible with 1.0.0.
bors merge |
Build succeeded: |
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR backports #7301, splitting integration/unit tests from each other. The conflicts were all with the last commit, where it had trouble merging when we changed the
parent/pom.xml
path with the different version.Additionally, it seems like the
RollingUpdateTest
was not backported yet - I let it be added, as it's still useful to test with patch releases, but I'm happy to split this into another PR if you'd like.Related issues
backports #7241
Definition of Done
Not all items need to be done depending on the issue and the pull request.
Code changes:
backport stable/0.25
) to the PR, in case that fails you need to create backports manually.Testing:
Documentation: