Set up testing and wheels for Windows on GitHub Actions #8131

mehrdadn · 2020-04-22T13:05:58Z

Why are these changes needed?

Lays the groundwork for testing and wheel building for Windows on GitHub Actions.

Related issue number

#631

Checks

I've run scripts/format.sh to lint the changes in this PR.
I've included any doc changes needed for https://docs.ray.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)

AmplabJenkins · 2020-04-22T13:11:15Z

Can one of the admins verify this patch?

AmplabJenkins · 2020-04-22T14:30:28Z

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

AmplabJenkins · 2020-04-22T15:19:15Z

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

AmplabJenkins · 2020-04-22T22:24:30Z

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

AmplabJenkins · 2020-04-22T22:33:19Z

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

AmplabJenkins · 2020-04-22T22:36:20Z

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

AmplabJenkins · 2020-04-22T23:02:10Z

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

pcmoritz · 2020-04-22T23:50:55Z

ci/travis/install-dependencies.sh

+    pip install flake8==3.7.7 flake8-comprehensions flake8-quotes==2.0.0 yapf==0.23.0  # Python linters
+    # readthedocs has an antiquated build env.
+    # This is a best effort to reproduce it locally to avoid doc build failures and hidden errors.
+    local filename


why is the below change needed? pip install seems to have worked before just fine

The sed line I can probably remove (I think I added it a while ago to help me debug some stuff), but the filter_compatible_pip_requirements line is needed since the blist requirement doesn't work on on Windows as I mentioned below.

I think we can remove it from the requirements-doc, because we are mocking it for read the docs anyways. Does that work?

Not sure, I can give it a shot and see.

Yeah, I'm pretty sure it will work :)

pcmoritz · 2020-04-22T23:53:19Z

ci/travis/install-dependencies.sh

+}
+
+# Takes a list of pip requirements and prints the ones that don't have known incompatibilities with the current system.
+filter_compatible_pip_requirements() {


Let's remove this. We can for example address it later by trying to remove the dependencies that are incompatible.

Wouldn't it just break something if I remove them from the requirements file? I think one of them was blist, I forget if there was another one. Should I just delete those lines from the requirements? I would've guessed it would break something for users.

AmplabJenkins · 2020-04-22T23:53:51Z

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

pcmoritz · 2020-04-23T00:02:50Z

ci/travis/ci.sh

+  esac
+  case "${HOSTTYPE}" in
+    x86_64) architecture=amd64;;
+    i?86*) architecture=i386;;


Why do we need to support that?

I guess we don't; I just added the case to make it clear there's a translation from HOSTTYPE and then this seemed like a cheap line to add afterwards. I can remove it.

AmplabJenkins · 2020-04-23T13:19:41Z

Merged build finished. Test PASSed.

AmplabJenkins · 2020-04-23T13:19: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/25100/
Test PASSed.

AmplabJenkins · 2020-04-26T14:03:57Z

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

…factor-4

pcmoritz · 2020-04-26T20:45:56Z

.travis.yml


  # random python tests TODO(ekl): these should be moved to bazel
  - if [ $RAY_CI_PYTHON_AFFECTED == "1" ]; then python -m pytest -v --durations=5 --timeout=300 python/ray/experimental/test/async_test.py; fi

  # bazel python tests. This should be run last to keep its logs at the end of travis logs.
-  - if [ $RAY_CI_PYTHON_AFFECTED == "1" ]; then ./ci/keep_alive bazel test --config=ci --test_tag_filters=-jenkins_only python/ray/tests/...; fi
-  - if [ $RAY_CI_TUNE_AFFECTED == "1" ]; then ./ci/keep_alive bazel test --config=ci --test_tag_filters=-jenkins_only python/ray/tune/...; fi
+  - if [ $RAY_CI_PYTHON_AFFECTED == "1" ]; then bazel_targets+=(python/ray/tests/...); fi


Why is this change needed?

It factors out and deduplicates the bazel test calls for maintainability.

No, it makes it harder to read (and harder to copy and paste into a terminal if you want to run it).

pcmoritz · 2020-04-26T21:00:02Z

ci/travis/ci.sh

  fi
-  test -z "${set_x}" || set -x  # restore set -x
+
+  # Deduplicate PATH


This should not be needed...

What are you basing this on?

reload_env can (and I believe is) called multiple times in a single shell, and it needs to be idempotent. If you change PATH n times without deduplicating then you're going to build with Bazel n times. I don't think that's a good idea.

AmplabJenkins · 2020-04-26T21:38:19Z

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

AmplabJenkins · 2020-04-26T23:01:28Z

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

AmplabJenkins · 2020-04-27T00:25:59Z

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

AmplabJenkins · 2020-04-28T15:16:36Z

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

AmplabJenkins · 2020-04-28T16:19:42Z

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

pcmoritz · 2020-04-28T17:57:42Z

.travis.yml

@@ -150,8 +122,7 @@ matrix:
        - . ./ci/travis/ci.sh build
      script:
        - . ./ci/travis/ci.sh preload
-        # Explicitly sleep 60 seconds for logs to go through
-        - ./ci/travis/test-wheels.sh || { cat /tmp/ray/session_latest/logs/* && sleep 60 && false; }
+        - . ./ci/travis/ci.sh run


This should be . ./ci/travis/ci.sh test_wheels

pcmoritz · 2020-04-28T17:58:09Z

.travis.yml

@@ -167,8 +138,7 @@ matrix:
        - . ./ci/travis/ci.sh build
      script:
        - . ./ci/travis/ci.sh preload
-        # Explicitly sleep 60 seconds for logs to go through
-        - ./ci/travis/test-wheels.sh || { cat /tmp/ray/session_latest/logs/* && sleep 60 && false; }
+        - . ./ci/travis/ci.sh run


This should be . ./ci/travis/ci.sh test_wheels

pcmoritz · 2020-04-28T17:59:02Z

.travis.yml

@@ -297,7 +267,7 @@ matrix:
        - . ./ci/travis/ci.sh build
      script:
        - . ./ci/travis/ci.sh preload
-        - bazel test --config=ci //cpp:all --build_tests_only --test_output=streamed
+        - . ./ci/travis/ci.sh run


This should be . ./ci/travis/ci.sh test_cpp

AmplabJenkins · 2020-04-29T09:41:24Z

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

AmplabJenkins · 2020-04-29T10:14:28Z

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

mehrdadn force-pushed the travis-refactor-4 branch from 830077a to 230754a Compare April 22, 2020 13:43

web-flow added 7 commits April 22, 2020 13:42

Define run()

0db8894

Move C++ worker tests into ci.sh

76898f9

Move some Java tests into ci.sh

e0eeb0f

Prepare to move Python tests into ci.sh

c898968

Reload environment for GitHub Actions

6aa65d5

Fix issues in install-dependencies.sh

7088479

Move wheels to ci.sh and fix related issues

12fb712

mehrdadn force-pushed the travis-refactor-4 branch 5 times, most recently from 11a3353 to 41ee482 Compare April 22, 2020 22:13

web-flow added 4 commits April 22, 2020 15:43

Don't bypass failures in install-ray.sh anymore

99cebcf

Make CI a little quieter

4380c7b

Move linting into ci.sh

eeca1b1

Add vitals test right after build

b773fab

mehrdadn force-pushed the travis-refactor-4 branch from 41ee482 to b773fab Compare April 22, 2020 22:48

pcmoritz self-requested a review April 22, 2020 23:45

pcmoritz reviewed Apr 22, 2020

View reviewed changes

pcmoritz reviewed Apr 23, 2020

View reviewed changes

bc06989

web-flow added 2 commits April 26, 2020 13:12

Merge remote-tracking branch 'remotes/upstream/master' into travis-re…

fa03cc9

…factor-4

Fix os.uname() unavailability on Windows

1535bbd

mehrdadn force-pushed the travis-refactor-4 branch from 944aefb to 1535bbd Compare April 26, 2020 20:17

pcmoritz reviewed Apr 26, 2020

View reviewed changes

web-flow added 2 commits April 26, 2020 14:34

d1605a3

ddb2d7d

6108ebf

mehrdadn marked this pull request as ready for review April 27, 2020 01:45

cb623f4

mehrdadn force-pushed the travis-refactor-4 branch from dfc3235 to cb623f4 Compare April 28, 2020 15:02

pcmoritz reviewed Apr 28, 2020

View reviewed changes

web-flow added 2 commits April 29, 2020 01:15

9ed72bb

d5a9578

pcmoritz approved these changes Apr 30, 2020

View reviewed changes

pcmoritz merged commit 254b1ec into ray-project:master Apr 30, 2020

mehrdadn deleted the travis-refactor-4 branch April 30, 2020 04:25

Set up testing and wheels for Windows on GitHub Actions #8131

Set up testing and wheels for Windows on GitHub Actions #8131

Conversation

mehrdadn commented Apr 22, 2020 • edited Loading

Why are these changes needed?

Related issue number

Checks

AmplabJenkins commented Apr 22, 2020

AmplabJenkins commented Apr 22, 2020

AmplabJenkins commented Apr 22, 2020

AmplabJenkins commented Apr 22, 2020

AmplabJenkins commented Apr 22, 2020

AmplabJenkins commented Apr 22, 2020

AmplabJenkins commented Apr 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pcmoritz Apr 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Apr 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Apr 23, 2020

AmplabJenkins commented Apr 23, 2020

AmplabJenkins commented Apr 26, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pcmoritz Apr 26, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mehrdadn Apr 26, 2020 • edited Loading

Choose a reason for hiding this comment

AmplabJenkins commented Apr 26, 2020

AmplabJenkins commented Apr 26, 2020

AmplabJenkins commented Apr 27, 2020

AmplabJenkins commented Apr 28, 2020

AmplabJenkins commented Apr 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmplabJenkins commented Apr 29, 2020

AmplabJenkins commented Apr 29, 2020

mehrdadn commented Apr 22, 2020 •

edited

Loading

pcmoritz Apr 26, 2020 •

edited

Loading

pcmoritz Apr 26, 2020 •

edited

Loading

mehrdadn Apr 26, 2020 •

edited

Loading