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

build: add daily/on-demand internet test workflow #40086

Closed
wants to merge 2 commits into from
Closed

build: add daily/on-demand internet test workflow #40086

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
19311e0
build: add daily/on-demand internet test workflow
Trott Sep 12, 2021
22cb29f
fixup! build: add daily/on-demand internet test workflow
Trott Sep 12, 2021
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
26 changes: 26 additions & 0 deletions .github/workflows/test-internet.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
name: test-internet

on:
workflow_dispatch:
schedule:
- cron: "5 0 * * *"
Comment on lines +3 to +6
Copy link
Contributor

@Mesteery Mesteery Sep 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Added it as a daily/on-demand workflow for now. We can later improve it to run on pull requests and pushes when appropriate only (or not).

I want to say that it's almost useless if you do it that way. It would just be impossible to run this workflow on a PR that changes the internet tests, and that's a shame, it loses all its usefulness IMO.

Suggested change
on:
workflow_dispatch:
schedule:
- cron: "5 0 * * *"
on:
workflow_dispatch:
schedule:
- cron: '5 0 * * *'
pull_request:
types: [opened, synchronize, reopened, ready_for_review]
paths:
- test/internet/**
- deps/cares/**
- lib/dns.js
- lib/internal/dns/**
- src/cares*
- lib/dgram.js
- lib/internal/dgram.js
- src/*udp*
- lib/*http*
- lib/internal/http.js
- lib/internal/http2/**
- src/*http*
- src/tcp*
- deps/ng*/**
push:
paths:
- test/internet/**
- deps/cares/**
- lib/dns.js
- lib/internal/dns/**
- src/cares*
- lib/dgram.js
- lib/internal/dgram.js
- src/*udp*
- lib/*http*
- lib/internal/http.js
- lib/internal/http2/**
- src/*http*
- src/tcp*
- deps/ng*/**
branches:
- master
- main
- canary
- v[0-9]+.x-staging
- v[0-9]+.x

Copy link
Member Author

@Trott Trott Sep 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The additions you're proposing here may be worth doing, but we should probably keep it on a schedule too. Other changes may break internet tests (such as changes to cares or dns.js) and we might not notice for a long time. If it's a month later, it will be very useful to have daily results to go back to so we can immediately narrow the issue down to a single day's worth of commits.

My idea was that someone can manually launch the job on PRs if the PR branch is in the repo. (Or they can launch it on their own branch if it's in their repo.)

Copy link
Member Author

Choose a reason for hiding this comment

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

By the way, the "run once a day and run it manually" mirrors what we do on Jenkins now, so they idea is that this would allow us to get rid of the Jenkins job.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Although it does seem like the Jenkins job runs much faster. Maybe I'm doing something wrong in the workflow and there's a way to have a persistent compile cache to speed things up or something.)

Copy link
Contributor

Choose a reason for hiding this comment

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

I have updated the suggestion.

Copy link
Member Author

Choose a reason for hiding this comment

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

If it's OK with you, I think my preference is to land this as-is and then incrementally add these files/paths over time. We've gone years and years without having these jobs run automatically, and there's always been certain concerns around running them routinely in CI. These concerns (don't want to hit a DNS, web, or other external service too much from our CI, for example) may not be well-founded, but just in case, I think an incremental approach is still the way to go.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's OK with me.


env:
PYTHON_VERSION: 3.9
FLAKY_TESTS: dontcare

jobs:
test-internet:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- name: Set up Python ${{ env.PYTHON_VERSION }}
uses: actions/setup-python@v2
with:
python-version: ${{ env.PYTHON_VERSION }}
- name: Environment Information
run: npx envinfo
- name: Build
run: make build-ci -j2 V=1 CONFIG_FLAGS="--error-on-warn"
- name: Test Internet
run: make test-internet -j2 V=1;