You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/01/24 04:37:48 UTC
[GitHub] [druid] suneet-s opened a new pull request #10791: Run integration tests in a second stage
suneet-s opened a new pull request #10791:
URL: https://github.com/apache/druid/pull/10791
### Description
Running all the tests in Travis is expensive. This PR attempts to split the tests into 2 stages so that the integration tests run only if the other tests were successful. This will allow more builds to run in parallel since the first stage tends to be faster than the integration tests (except for the intelliJ inspection check job)
<hr>
This PR has:
- [ ] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [ ] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/licenses.yaml)
- [ ] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [ ] been tested in a test Druid cluster.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s merged pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
suneet-s merged pull request #10791:
URL: https://github.com/apache/druid/pull/10791
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #10791:
URL: https://github.com/apache/druid/pull/10791#issuecomment-767156772
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis edited a comment on pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
clintropolis edited a comment on pull request #10791:
URL: https://github.com/apache/druid/pull/10791#issuecomment-767164380
I think 2 stages is good, where the first stage is mainly to shake out style, inspection, spelling, compile, unit tests failures, and test coverage before opening the flood gates to the rest of them. I worry that 3 stages would potentially increase wall time too much.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #10791:
URL: https://github.com/apache/druid/pull/10791#discussion_r564083590
##########
File path: .travis.yml
##########
@@ -46,6 +45,7 @@ install: MAVEN_OPTS='-Xmx3000m' travis_wait 15 ${MVN} clean install -q -ff ${MAV
stages:
- name: test # jobs that do not specify a stage get this default value
if: type != cron
+ - name: it
Review comment:
nit: should we call this 'integration-tests' or something so it looks a bit more descriptive on web stuffs?
<img width="1231" alt="Screen Shot 2021-01-25 at 2 20 45 PM" src="https://user-images.githubusercontent.com/1577461/105773297-8929bd80-5f18-11eb-84b4-8821a66f4b5f.png">
although I do kind of like it like this too, so I'm +1 on this PR either way
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on a change in pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
clintropolis commented on a change in pull request #10791:
URL: https://github.com/apache/druid/pull/10791#discussion_r564083590
##########
File path: .travis.yml
##########
@@ -46,6 +45,7 @@ install: MAVEN_OPTS='-Xmx3000m' travis_wait 15 ${MVN} clean install -q -ff ${MAV
stages:
- name: test # jobs that do not specify a stage get this default value
if: type != cron
+ - name: it
Review comment:
nit: should we call this 'integration-tests' or something so it looks a bit more descriptive on web stuffs?
<img width="1231" alt="Screen Shot 2021-01-25 at 2 20 45 PM" src="https://user-images.githubusercontent.com/1577461/105773297-8929bd80-5f18-11eb-84b4-8821a66f4b5f.png">
although I do kind of like it like this too, so I'm +1 on this PR either way
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on a change in pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #10791:
URL: https://github.com/apache/druid/pull/10791#discussion_r565016164
##########
File path: .travis.yml
##########
@@ -46,6 +45,7 @@ install: MAVEN_OPTS='-Xmx3000m' travis_wait 15 ${MVN} clean install -q -ff ${MAV
stages:
- name: test # jobs that do not specify a stage get this default value
if: type != cron
+ - name: it
Review comment:
I like your suggestions. Trying to find the right balance and the right name for the stages is tough.
I could move all the jdk 11 tests into the second stage, but then I can't think of what the name should be. I thought about slow / fast. But right now `intelliJ inspections` and the `indexing modules tests` take ~ 40 minutes, which is actually longer than most integration tests 😅
I think this split is good enough as a starting point, but would be extra happy if someone else proposed a more optimal split.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #10791:
URL: https://github.com/apache/druid/pull/10791#issuecomment-767162872
> I guess we could also potentially push the jdk11 unit tests into the 2nd stage (or the jdk8 tests?), since ideally tests should be identical in both environments unless something that isn't portable has been done, which I think is relatively rare
Ah that's a good idea. The integration tests are known to be flaky, eg. #10691 so I didn't want to split them up that way. But maybe it's worth splitting up the unit tests.
Let me know what you think, and I'll be happy to change it either way: Should it be 3 stages: jdk8 unit tests > integration tests > jdk 11 unit tests? Or just 2 stages with jdk 11 tests and integration tests running in the second stage?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #10791:
URL: https://github.com/apache/druid/pull/10791#issuecomment-767156772
I guess we could also potentially push the jdk11 unit tests into the 2nd stage (or the jdk8 tests?), since ideally tests should be identical in both environments unless something that isn't portable has been done, which I think is relatively rare
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis edited a comment on pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
clintropolis edited a comment on pull request #10791:
URL: https://github.com/apache/druid/pull/10791#issuecomment-767164380
I think 2 stages is good, where the first stage is mainly to shake out style, inspection, spelling, compile, unit tests failures, and test coverage before opening the flood gates to the rest of them. I worry that 3 stages would potentially increase wall time too much.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] suneet-s commented on pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #10791:
URL: https://github.com/apache/druid/pull/10791#issuecomment-767162872
> I guess we could also potentially push the jdk11 unit tests into the 2nd stage (or the jdk8 tests?), since ideally tests should be identical in both environments unless something that isn't portable has been done, which I think is relatively rare
Ah that's a good idea. The integration tests are known to be flaky, eg. #10691 so I didn't want to split them up that way. But maybe it's worth splitting up the unit tests.
Let me know what you think, and I'll be happy to change it either way: Should it be 3 stages: jdk8 unit tests > integration tests > jdk 11 unit tests? Or just 2 stages with jdk 11 tests and integration tests running in the second stage?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] clintropolis commented on pull request #10791: Run integration tests in a second stage
Posted by GitBox <gi...@apache.org>.
clintropolis commented on pull request #10791:
URL: https://github.com/apache/druid/pull/10791#issuecomment-767164380
I think 2 stages is good, where the first stage is to mainly to shake out style, inspection, spelling, compile, unit tests failures, and test coverage before opening the flood gates to the rest of them. I worry that 3 stages would potentially increase wall time too much.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org