-
Notifications
You must be signed in to change notification settings - Fork 79
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
CI: Test across Node 16/18/20 + arm64 MacOS test #444
Conversation
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.
Nice work! Left a bunch of comments + suggestions.
@@ -8,64 +8,30 @@ on: | |||
|
|||
jobs: | |||
build-and-test-osx: | |||
runs-on: macos-latest | |||
runs-on: ${{ matrix.os }} |
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.
AWESOME!! I had been meaning to do this for ages, but never got around to figuring out what the syntax was.
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 have been making some mental foo of late, you can generate matrices dynamically in your actions, so I have a run of pact ffi/ pact verifier cli and pact mock server cli, for all the targets, in one super clean action.
I just wish you could report on individual matrix combos, because you could build up an awesome compat matrix, with github badges.
The other way is to have separate workflows , I need to check if you can pull in one master workflow and then have loads of mini workflows, that just specify x combo (node 14 / windows) so they appear as seperate workflows, and you could get a status badge for each one.
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 just wish you could report on individual matrix combos, because you could build up an awesome compat matrix, with github badges.
I dunno - I think it's better to just report the whole success / failure - as then you don't get tempted to continue with "well, it doesn't work on everything we deploy to".
It would be nice to be able to generate the list of tested combinations, though!
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 whole just fail isn’t helpful for me finding the needle in the haystack and github’s view of multi matrix runs needs some work, detail gets lots and you just have 12 boxes that al look the same from the truncated description until you click on one, and that is an expensive use of time
i need to check out github blocks again as there is meant to be some cool stuff you can do in readmes now
appreciate the 👀 buddy ❤️ |
const isDarwinArm64 = process.platform === 'darwin' && process.arch === 'arm64'; | ||
const isLinuxArm64 = process.platform === 'linux' && process.arch === 'arm64'; | ||
const isCirrusCi = process.env['CIRRUS_CI'] === 'true'; | ||
const usesOctetStream = |
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.
This makes me sad. Not your fault of course. We need to find a solution to this!
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 know, I'll card this up for a yak shave, now have we have a way of testing repeatably across platform, we can sort it, or document it
download-libs.sh
otherwise it asks the user to confirm everytime. 🤯npm ci
when running locallySmells
Cross CI provider and cross arch has shown a few inconsistencies in content-type matching, which is clearly demonstrable here. Haven't thought of a solution just yet, but this at least highlights and allows us to test more combinations than before
See pact-foundation/pact-reference#171 for why we have an OS switch here
Previous state
Current state