-
Notifications
You must be signed in to change notification settings - Fork 68
Fix: more versioning around nightly and maint/main
#359
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
Conversation
1. stop trying to "coerce" versions on input 2. handle `OTP-` in one more place 3. handle `nightly`
74737e6 to
03120e3
Compare
| - otp-version: '27' | ||
| os: 'macos-15' | ||
| version-type: 'strict' | ||
| - otp-version: '26' | ||
| os: 'macos-15' | ||
| version-type: 'strict' |
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 shoulda been failing but wasn't. Guess it wasn't as strict as we intended...
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.
Oh, it was getting calculated as maint-<otp-version>.
23af438 to
127dcbd
Compare
test/setup-beam.test.js
Outdated
| it('is Ok for known linux version', async () => { | ||
| before = simulateInput('version-type', 'strict') | ||
| spec = '26' | ||
| spec = 'maint-26' |
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 made no sense, but now we see it. Why would we want 26 to strict to be maint-26? These days this is opt-in, so I think some sanity was restored.
There is no strict 27 for macos-15, for example
127dcbd to
3ecdae9
Compare
test/setup-beam.test.js
Outdated
| spec = 'main-otp-28' | ||
| otpVersion = 'maint-28' | ||
| expected = 'main-otp-28' | ||
| await setupBeam.installOTP(otpVersion) | ||
| got = await setupBeam.getElixirVersion(spec, otpVersion) | ||
| assert.deepStrictEqual(got, expected) |
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.
Here's the test for #358.
| spec = 'nightly' | ||
| otpVersion = '28' | ||
| expected = 'nightly' | ||
| got = await setupBeam.getGleamVersion(spec, otpVersion) | ||
| assert.deepStrictEqual(got, expected) |
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.
Here's the test for #356.
|
@lpil (the |
definitely, I would considered that a bug that |
We're not testing it on Windows, I believe. I'll see about adding a test to verify the new behavior. |
|
Using this PR, - elixir-version: "1.18.4"
otp-version: "26"
version-type: 'strict'resolves to: Is that expected? |
|
I think the strict should resolve exactly to input (give or take a |
We'll test once per platform in similar conditions
|
Agreed |
9b94d01 to
b44ebc6
Compare
b44ebc6 to
f984a85
Compare
|
Is it possible you forgot Edit: even if you didn't and I'm not seeing something, I'm pushing code so that we have a validation that strict is strict (alongside an exception for the case you mentioned - which unfortunately, at least at the moment, we can't be too strict on, until we wanna parse the error message). |
|
Yes that's what happened, my workflow file was wrong, apologies for any confusion. https://github.com/wojtekmach/setup_beam_bug/actions/runs/16078422072/workflow |
f984a85 to
0b1e9cb
Compare
|
No prob. As stated, if we have tests for specific situations we have less doubts. |
| .forEach((line) => { | ||
| const otpMatch = line | ||
| .match(/^([^ ]+)?( .+)/)[1] | ||
| .match(/^([^-]+-)?(.+)$/) |
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 one goes away.
|
I'm merging, for release, in the hopes:
|
|
This was released in v1.20.4. |
Description
This'll address two issues:
main/maint)nightlyI'll start by pushing tests that show the issue and then a fix that shouldn't affect any other tests.
Issue 1: https://github.com/erlef/setup-beam/actions/runs/16057710018/job/45316130218#step:5:54
Issue 2: https://github.com/erlef/setup-beam/actions/runs/16057710018/job/45316130218#step:5:67
Closes #356.
Closes #358.
26(ifstrict) was getting calculated asmaint-26. It's no longer the case, but I think we can live with that (?)