test: otp branch matching

[grzuy]

Description

Hi,

Failing test showing OTP branch matching no longer working as of 1.17. This test is passing for me in 1.16.

Is that intentional or a regression?

Thanks!

• [x] I have performed a self-review of my changes
• [x] I have read and understood the contributing guidelines

[paulo-ferraz-oliveira]

OTP branch matching no longer working

I didn't look at this yet, but is the whole matching mechanism broken? Most of the tests should be passing. What specific combo does not work for you?

CI didn't run for this pr, I don't know why, but there's no fix in your code, really, so 🤷.

@josevalim, would you have a combo I can use for testing and maybe fix on top of that later?

[paulo-ferraz-oliveira]

@grzuy, replying to your question: there was no explicit intention, otherwise we'd have bump'ed the major version, so this needs looking at.

[grzuy]

@grzuy, replying to your question: there was no explicit intention, otherwise we'd have bump'ed the major version, so this needs looking at.

Gotcha.

I didn't look at this yet, but is the whole matching mechanism broken? Most of the tests should be passing. What specific combo does not work for you?

None of the existing tests failing, only this one added in this PR failing locally, which is added to add coverage for this.

CI didn't run for this pr, I don't know why,

Neither do I.

but there's no fix in your code, really, so 🤷.

Nope, sorry for the confusion. Intention was to at least provide a failing test case for the apparent bug. Haven't had time yet to work on a fix. Also wanted to confirm it was a bug and it wasn't intentional before spending time fixing.

THanks @paulo-ferraz-oliveira

@josevalim, would you have a combo I can use for testing and maybe fix on top of that later?

[paulo-ferraz-oliveira]

Cool. I'm looking at this now. Hopefully it's not complicated to solve.

[paulo-ferraz-oliveira]

I see this fail, but with a reasonable error message (should you be using option 'version-type': 'strict'?); there's possibly a regression in terms of expectations, though.

Should we add exceptions to those? (main, maint, master, ...)

Edit: I don't know the actual use case...

Edit: fwiw, we were not testing for the use case of maint/master which is why this difference was introduced.

[josevalim]

We use both maint and master in Elixir. I will be glad to change our CI config if that’s the best course of action.

[paulo-ferraz-oliveira]

I understand. But is it possible somebody else is doing it too (using those branches)?

To recap:

1. it wasn't intentional
2. it wasn't being tested (which is why it slid through the cracks of review)
3. if it affects only elixir-lang I wouldn't mind you adapt to it, but at the same time I don't wanna force this since it's a valid use case

I'll push a pull request, ping you and also @starbelly, for thoughts. I have a simple working solution locally already, just testing some stuff out...

[paulo-ferraz-oliveira]

Closing this in favour of #242. Thanks for the report.

[grzuy]

Thanks to you :raised_hands: