[Testing] Do not run any non-RLlib/core tests if only RLLib affected (except wheels). #7892

sven1977 · 2020-04-03T19:03:57Z

Non-wheel & non-RLlib travis test jobs will not(!) be run if only RLLIB_AFFECTED.

Reasoning: If only RLlib files have changed, only RLlib tests should be re-run (and the wheels should be rebuilt/updated).

We already have in place: If only non-RLlib files have changed, only the RLlib learning tests are run to assure all our RL-algos are still learning.

This will significantly reduce the stress currently put on our travis resources.

Related issue number

Checks

I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://ray.readthedocs.io/en/latest/.
I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failure rates at https://ray-travis-tracker.herokuapp.com/.
Testing Strategy
- Unit tests
- Release tests
- This PR is not tested (please justify below)

…r generating the 2 wheels (OSX and Linux).

AmplabJenkins · 2020-04-03T19:10:38Z

Can one of the admins verify this patch?

…_test_all_of_ray_core_for_rllib_only_changes � Conflicts: � rllib/utils/__init__.py

AmplabJenkins · 2020-04-03T20:28:44Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24209/
Test FAILed.

AmplabJenkins · 2020-04-03T20:35:22Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24208/
Test PASSed.

AmplabJenkins · 2020-04-03T20:49:46Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24210/
Test FAILed.

AmplabJenkins · 2020-04-03T21:18:25Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24211/
Test PASSed.

…min barrier).

…hanges

…f_ray_core_for_rllib_only_changes

AmplabJenkins · 2020-04-04T13:24:55Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24232/
Test FAILed.

AmplabJenkins · 2020-04-04T13:36:08Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24233/
Test PASSed.

AmplabJenkins · 2020-04-04T15:02:46Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24238/
Test PASSed.

…_test_all_of_ray_core_for_rllib_only_changes

simon-mo · 2020-04-06T17:01:59Z

.travis.yml

@@ -56,16 +56,17 @@ matrix:
        - RAY_GCS_SERVICE_ENABLED=false
      install:
        - eval `python $TRAVIS_BUILD_DIR/ci/travis/determine_tests_to_run.py`
+        - if [ $RAY_CI_SERVE_AFFECTED != "1" ] && [ $RAY_CI_TUNE_AFFECTED != "1" ] && [ $RAY_CI_PYTHON_AFFECTED != "1" ] && [ $RAY_CI_STREAMING_CPP_AFFECTED != "1" ] && [ $RAY_CI_JAVA_AFFECTED != "1" ]; then exit; fi


this condition doesn't work. It should run when RAY_CI_JAVA_AFFECTED is 1

and it does, no?
It will NOT(!) exit the install, if RAY_CI_JAVA_AFFECTED=1

Then, in the script section: if [ $RAY_CI_JAVA_AFFECTED == "1" ]; then ./java/test.sh; fi

simon-mo · 2020-04-06T17:03:12Z

.travis.yml

@@ -76,13 +77,15 @@ matrix:
        - RAY_GCS_SERVICE_ENABLED=false
      install:
        - eval `python $TRAVIS_BUILD_DIR/ci/travis/determine_tests_to_run.py`
+        - if [ $RAY_CI_PYTHON_AFFECTED != "1" ]; then exit; fi


for gcs_service test, we actually want to run it on every python changes

Oh, so you mean, it should run even if only RAY_CI_RLLIB_AFFECTED=1?

AmplabJenkins · 2020-04-06T17:24:04Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24304/
Test PASSed.

sven1977 · 2020-04-06T19:00:23Z

Let me create another var, which is 1 iff only RLLIB is affected (and nothing else).
That should simplify the if blocks and make everything less confusing.

…ray-core stuff (except wheels) if only RLlib changed.

simon-mo · 2020-04-06T20:08:35Z

@mehrdadn can you help take a look at this PR as well? mostly verify the logic.

AmplabJenkins · 2020-04-06T20:40:48Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24314/
Test PASSed.

mehrdadn

The logic looks fine as far as I can tell @simon-mo. I'm a bit concerned it's becoming more brittle and getting increasingly difficult to reason about the testing logic. For example, the conditions for defining RAY_CI_ONLY_RLLIB_AFFECTED form an almost-exhaustive list of exclusions, which will likely go out of sync later. Or for example, I think it'll become confusing later what "only" means in RAY_CI_ONLY_RLLIB_AFFECTED, since wheels are also deemed affected here. I think right now it's clear that it means "only" with respect to the phase of compiling source code, but that will likely not stay so simple down the road.

We can (probably should) merge this for now to get builds through, but we should seriously consider a more scalable and robust solution later. I think the solution here would be using Bazel for everything, so that it can figure out dependencies for tests on its own (and we can insert our own scripts into the process to help it wherever needed). Probably something I can look into once I'm done with the higher priority stuff on the Windows side.

simon-mo · 2020-04-09T03:44:13Z

Thanks @mehrdadn! Sounds good, moving the dependency analysis to bazel make sense to me. Probably through some bazel query tool we can determine which sets of tests are necessary.

sven1977 · 2020-04-09T12:13:49Z

Trying a re-test with only Rllib changes.

sven1977 · 2020-04-09T12:15:00Z

I agree, it's probably better to take what-needs-to-be-tested-logic out of travis.yaml and move it into BAZEL.

AmplabJenkins · 2020-04-09T16:02:52Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24449/
Test FAILed.

…_test_all_of_ray_core_for_rllib_only_changes

sven1977 · 2020-04-09T20:41:26Z

Hey @simon-mo Could you merge? Tests are all ok, except for the known broken ones (tmp file creation currently broken: rllib/tests/test_io and test_tempfile.py).

mehrdadn · 2020-04-09T20:43:09Z

Yes please merge :) I'm doing surgery on CI so it'll make one of our lives very difficult if this isn't merged soon!

AmplabJenkins · 2020-04-09T20:56:25Z

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Ray-PRB/24469/
Test FAILed.

…(except wheels). (ray-project#7892) * Do not run any non-RLlib/core tests if only RLLib affected, except for generating the 2 wheels (OSX and Linux). * Test noop RLlib change. * Test noop RLlib change. * Fix broken RLlib tests in master. * Split BAZEL learning tests into cartpole and pendulum (reached the 60min barrier). * Fix error_outputs option in BAZEL for RLlib regression tests. * Fix. * Test. * WIP. * Add env flag RAY_CI_ONLY_RLLIB_AFFECTED to refrain from testing most ray-core stuff (except wheels) if only RLlib changed. * Test RLlib-only change.

sven1977 added 2 commits April 3, 2020 20:57

Do not run any non-RLlib/core tests if only RLLib affected, except fo…

7e5c4e9

…r generating the 2 wheels (OSX and Linux).

Test noop RLlib change.

24d23ae

sven1977 added 2 commits April 3, 2020 21:43

Test noop RLlib change.

81ae7a8

Merge branch 'master' of https://github.com/ray-project/ray into dont…

e2b4fee

…_test_all_of_ray_core_for_rllib_only_changes � Conflicts: � rllib/utils/__init__.py

sven1977 added 6 commits April 4, 2020 10:51

Fix broken RLlib tests in master.

c7b9c44

Split BAZEL learning tests into cartpole and pendulum (reached the 60…

e1e1894

…min barrier).

Fix error_outputs option in BAZEL for RLlib regression tests.

6ddee3c

Merge branch 'master' into dont_test_all_of_ray_core_for_rllib_only_c…

6c496e7

…hanges

Fix.

c760a5f

Merge branch 'fix_failing_rllib_tests_in_master' into dont_test_all_o…

f0fd937

…f_ray_core_for_rllib_only_changes

Test.

fc116f0

sven1977 added 2 commits April 6, 2020 17:54

Merge branch 'master' of https://github.com/ray-project/ray into dont…

64945aa

…_test_all_of_ray_core_for_rllib_only_changes

WIP.

be2f78b

simon-mo reviewed Apr 6, 2020

View reviewed changes

Add env flag RAY_CI_ONLY_RLLIB_AFFECTED to refrain from testing most …

93148ef

…ray-core stuff (except wheels) if only RLlib changed.

simon-mo approved these changes Apr 6, 2020

View reviewed changes

simon-mo requested a review from mehrdadn April 9, 2020 03:21

mehrdadn approved these changes Apr 9, 2020

View reviewed changes

Test RLlib-only change.

afe9cca

Merge branch 'master' of https://github.com/ray-project/ray into dont…

42f6262

…_test_all_of_ray_core_for_rllib_only_changes

sven1977 added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Apr 9, 2020

simon-mo merged commit 0a5b6d1 into ray-project:master Apr 9, 2020

sven1977 deleted the dont_test_all_of_ray_core_for_rllib_only_changes branch August 21, 2020 07:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Testing] Do not run any non-RLlib/core tests if only RLLib affected (except wheels). #7892

[Testing] Do not run any non-RLlib/core tests if only RLLib affected (except wheels). #7892

sven1977 commented Apr 3, 2020 •

edited

Loading

AmplabJenkins commented Apr 3, 2020

AmplabJenkins commented Apr 3, 2020

AmplabJenkins commented Apr 3, 2020

AmplabJenkins commented Apr 3, 2020

AmplabJenkins commented Apr 3, 2020

AmplabJenkins commented Apr 4, 2020

AmplabJenkins commented Apr 4, 2020

AmplabJenkins commented Apr 4, 2020

simon-mo Apr 6, 2020

sven1977 Apr 6, 2020

simon-mo Apr 6, 2020

sven1977 Apr 6, 2020

AmplabJenkins commented Apr 6, 2020

sven1977 commented Apr 6, 2020

simon-mo commented Apr 6, 2020

AmplabJenkins commented Apr 6, 2020

mehrdadn left a comment

simon-mo commented Apr 9, 2020

sven1977 commented Apr 9, 2020

sven1977 commented Apr 9, 2020

AmplabJenkins commented Apr 9, 2020

sven1977 commented Apr 9, 2020

mehrdadn commented Apr 9, 2020

AmplabJenkins commented Apr 9, 2020

[Testing] Do not run any non-RLlib/core tests if only RLLib affected (except wheels). #7892

[Testing] Do not run any non-RLlib/core tests if only RLLib affected (except wheels). #7892

Conversation

sven1977 commented Apr 3, 2020 • edited Loading

Related issue number

Checks

AmplabJenkins commented Apr 3, 2020

AmplabJenkins commented Apr 3, 2020

AmplabJenkins commented Apr 3, 2020

AmplabJenkins commented Apr 3, 2020

AmplabJenkins commented Apr 3, 2020

AmplabJenkins commented Apr 4, 2020

AmplabJenkins commented Apr 4, 2020

AmplabJenkins commented Apr 4, 2020

simon-mo Apr 6, 2020

Choose a reason for hiding this comment

sven1977 Apr 6, 2020

Choose a reason for hiding this comment

simon-mo Apr 6, 2020

Choose a reason for hiding this comment

sven1977 Apr 6, 2020

Choose a reason for hiding this comment

AmplabJenkins commented Apr 6, 2020

sven1977 commented Apr 6, 2020

simon-mo commented Apr 6, 2020

AmplabJenkins commented Apr 6, 2020

mehrdadn left a comment

Choose a reason for hiding this comment

simon-mo commented Apr 9, 2020

sven1977 commented Apr 9, 2020

sven1977 commented Apr 9, 2020

AmplabJenkins commented Apr 9, 2020

sven1977 commented Apr 9, 2020

mehrdadn commented Apr 9, 2020

AmplabJenkins commented Apr 9, 2020

sven1977 commented Apr 3, 2020 •

edited

Loading