-
Notifications
You must be signed in to change notification settings - Fork 126
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.
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
feat(.github): run QUIC Interop Runner #1682
Conversation
Run the QUIC Interop Runner testcases on pull requests when entering the merge queue.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1682 +/- ##
==========================================
+ Coverage 92.98% 93.01% +0.02%
==========================================
Files 120 120
Lines 37399 37399
==========================================
+ Hits 34775 34785 +10
+ Misses 2624 2614 -10 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ready for review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest duplicating this in mozilla/neqo
for now until quic-interop/quic-interop-runner#356 is merged.
client: neqo-latest,quic-go,ngtcp2,neqo,msquic | ||
server: neqo-latest,quic-go,ngtcp2,neqo,msquic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a strong opinion. Happy to change.
name: 'neqo-latest' | ||
image: ${{ steps.docker_build_and_push.outputs.imageID }} | ||
url: https://github.com/mozilla/neqo | ||
test: handshake |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testcase handshake
for now to keep CI time low. I suggest we expand gradually once we gained some experience running the QUIC Interop tests on each pull request. Happy to change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment with the test results on each pull requests will only appear once this workflow is in main
.
- run: docker image ls | ||
shell: bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debugging leftover?
sudo apt-get install -y wireshark tshark jq | ||
shell: bash | ||
|
||
- uses: actions/setup-python@v4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- uses: actions/setup-python@v4 | |
- uses: actions/setup-python@v5 |
shell: bash | ||
|
||
- name: Checkout quic-interop/quic-interop-runner repository | ||
uses: actions/checkout@v2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uses: actions/checkout@v2 | |
uses: actions/checkout@v4 |
- name: Install Python packages | ||
run: | | ||
cd quic-interop-runner | ||
pip install -U pip | ||
pip install -r requirements.txt | ||
shell: bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense to cache these? https://github.com/actions/setup-python?tab=readme-ov-file#caching-packages-dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea.
07fe740 enables caching. You can see it in action on https://github.com/mozilla/neqo/actions/runs/8176970683/job/22357558169?pr=1682.
Thanks for the review @larseggert. I addressed all comments. Mind taking another look? |
Run the QUIC Interop Runner testcases on pull requests when entering the merge queue.
This automation would prevent regressions (like e.g. #1676, #1627, #1578, #1714, ...) and continuously ensure interoperability with other implementations.
What do you think? Is this helpful?