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.

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

Run s2n tests for only one GHC version #1386

Closed
wants to merge 8 commits into from
Closed

Conversation

atomb
Copy link
Contributor

@atomb atomb commented Jul 21, 2021

This may not fix the timeout issues we're seeing, but at least it'll reduce the amount of work we're doing, and reduce the number of tests that might timeout.

Also fixes several other CI issues discovered along the way.

@robdockins
Copy link
Contributor

robdockins commented Jul 31, 2021

I don't know how easy/hard this is to do, but it would be nice if the s2n tests only depended on the particular build that they need in order to run. As it is now, they wait for all builds to successfully complete before beginning. In particular, if one of the builds fails because of some configuration situation, the proof tests don't run at all.

For example, the Windows 8.8.4 build currently seems like it might stall until it is canceled, and the proof tests will never run, preventing merging.

@atomb
Copy link
Contributor Author

atomb commented Aug 2, 2021

I entirely agree with that, but I unfortunately also don't know how hard it is to do. At the very least, it'll require learning some more about how GitHub Actions work.

@@ -167,7 +162,7 @@ jobs:
path: dist/bin
name: ${{ runner.os }}-bins

- if: "matrix.os == 'ubuntu-18.04'"
- if: "matrix.os == 'ubuntu-20.04'"
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to make this if: runner.os == 'Linux' so that it's immunized against future ubuntu release updates?

The concern here is that the matrix entries might be updated without checking the rest of the file for matches and suddenly this step silently stops being run.

@pnwamk pnwamk mentioned this pull request Aug 24, 2021
@pnwamk
Copy link
Contributor

pnwamk commented Aug 24, 2021

N.B., #1429 includes the fix from b66f637 in this PR (along with some other server-related CI fixes)

@atomb
Copy link
Contributor Author

atomb commented Sep 14, 2021

At this point, this PR is actually all about updating GHC and Z3 versions, which may not actually be something we want to do right now.

@atomb
Copy link
Contributor Author

atomb commented Oct 25, 2021

The important parts of this have been incorporated into other PRs, and the remainder has been re-done in better ways.

@atomb atomb closed this Oct 25, 2021
@RyanGlScott RyanGlScott deleted the at-s2n-8.10-only branch March 22, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants