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.

Sign up for GitHub

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

Add Python 3.9 for unit tests in CI (experimental, do not stop other builds if it fails) #3854

Merged
merged 2 commits into from
Nov 4, 2020
Merged

Add Python 3.9 for unit tests in CI (experimental, do not stop other builds if it fails) #3854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50c8a9a
Add Python 3.9 in CI (experimental)
kinow Oct 5, 2020
496c211
Try restricting memory of Docker containers for bash tests
kinow Oct 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/bash.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ jobs:

- name: Build Docker container
run: |
docker build -t bash:test dockerfiles/bash/
docker run --name bash -v $(pwd -P):/root/cylc-flow --rm -t -d bash:test /bin/bash
docker build -t bash:test -m 1G --memory-swap 1G dockerfiles/bash/
docker run --name bash -v $(pwd -P):/root/cylc-flow --rm -t -d -m 1G --memory-swap 1G bash:test /bin/bash
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was getting some errors due to low virtual memory when building the container. Not sure if others have seen this, but might be good to constraint resources so our builds don't fail sporadically.

docker ps -a

- name: Install Cylc dependencies in the container
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@ on: [pull_request]
jobs:
fast-tests:
runs-on: ubuntu-latest
continue-on-error: ${{ matrix.experimental }}
timeout-minutes: 15
strategy:
matrix:
python-version: ['3.7', '3.8']
experimental: [false]
include:
- python-version: 3.9
experimental: true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are python-version and experimental lists above, but scalar values here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, hadn't noticed it. Both appear to work fine. But let me edit that commit to match the other values 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, just need to wait for CI to run and confirm it works too.

Copy link
Member

@hjoliver hjoliver Oct 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I see the GH Actions docs does it the way you originally did it, so maybe it works either way 🤞 )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bugger:

Error: .github/workflows/test.yml (Line: 25, Col: 27): A sequence was not expected
Error: The template is not valid. .github/workflows/test.yml (Line: 25, Col: 27): A sequence was not expected

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But good to know! I guess if you have multiple versions, then you will have to do something like

- python-version: 1
  experimental: true
- python-version: 2
  experimental: true
...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the "experimental" mean exactly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a feature in GitHub actions that allows to mark a branch of the matrix-tree as not-ready I guess? It's really common in Java and JavaScript, for new versions of the JVM or of Node (which are experimental in relation to the code; meaning that they may not work, and the developers/CI are "experimenting" with it? 😄 )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, fair enough. Just metadata then, no real consequences 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can remove the experimental thing now 3.9 is out.

steps:
- name: Checkout
uses: actions/checkout@v2
Expand Down