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

STORM-2771: By default don't run any tests as integration tests #2368

Merged
merged 1 commit into from Oct 13, 2017

Conversation

@revans2
Copy link
Contributor

revans2 commented Oct 10, 2017

No description provided.

@srdo

This comment has been minimized.

Copy link
Contributor

srdo commented Oct 11, 2017

Can we avoid using the dummy annotation by setting http://maven.apache.org/surefire/maven-failsafe-plugin/integration-test-mojo.html#skipTests to true for the default plugin configuration, and setting it to false for the integration-tests-only and all-tests profiles?

@revans2

This comment has been minimized.

Copy link
Contributor Author

revans2 commented Oct 11, 2017

@srdo I'm not sure I can try it out and see.

@revans2

This comment has been minimized.

Copy link
Contributor Author

revans2 commented Oct 11, 2017

@srdo I did some tests and it works. The only problem with it is that if I do something odd like mvn clean install -Pall-tests -DskipTests it ends up running all of the java integration tests. I'm not sure if that is a big deal or not. I will try to fix it, but I am going to have to see if maven supports some kind of conditionals in their macro substitution.

@revans2

This comment has been minimized.

Copy link
Contributor Author

revans2 commented Oct 11, 2017

Maven does not support conditional properties. I would have to do some ugly things with a profile that is activated when a property is set, and I am not sure that even then I could make it work. Right now I like having the NoTests interface more than having skipTests not work in all situations. But that is just me, if others disagree I can change it, it is not a big deal to me.

@srdo

This comment has been minimized.

Copy link
Contributor

srdo commented Oct 11, 2017

Thanks for trying it out. I think we can get the same effect as the annotation by setting <java.integration.test.include>no.tests</java.integration.test.include> in the default properties. It's similar to how integration-tests-only disables the unit tests.

+1 for the fix, with or without the annotation.

@revans2 revans2 force-pushed the revans2:STORM-2771 branch from cd1ca56 to 278cf20 Oct 12, 2017
@revans2

This comment has been minimized.

Copy link
Contributor Author

revans2 commented Oct 12, 2017

@srdo Made the change you suggested and it works great, thanks. I squashed the commits and pushed as new version. Have a look.

@srdo

This comment has been minimized.

Copy link
Contributor

srdo commented Oct 12, 2017

Looks great, +1

@asfgit asfgit merged commit 278cf20 into apache:master Oct 13, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
asfgit pushed a commit that referenced this pull request Oct 13, 2017
… into STORM-2771

STORM-2771: By default don't run any tests as integration tests

This closes #2368
asfgit pushed a commit that referenced this pull request Oct 13, 2017
… into STORM-2771

STORM-2771: By default don't run any tests as integration tests

This closes #2368
asfgit pushed a commit that referenced this pull request Oct 13, 2017
… into STORM-2771

STORM-2771: By default don't run any tests as integration tests

This closes #2368
asfgit pushed a commit that referenced this pull request Oct 13, 2017
… into STORM-2771

STORM-2771: By default don't run any tests as integration tests

This closes #2368
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.