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

Eventually has a breaking change from 1.22.1 -> 1.23.0 #605

Open
KevinJCross opened this issue Nov 9, 2022 · 4 comments
Open

Eventually has a breaking change from 1.22.1 -> 1.23.0 #605

KevinJCross opened this issue Nov 9, 2022 · 4 comments

Comments

@KevinJCross
Copy link

hi,
There is a very subtle breaking change from 1.22.1 to version 1.23.0
The prototype of Eventually changed to some thing that is causing compile errors on old code.
The previous declaration was

func Eventually(actual interface{}, args ...interface{}) AsyncAssertion {

The declation in 1.23.0 is

func Eventually(args ...interface{}) AsyncAssertion {

These are explicitly different. The version in 1.22.1 requires at least one argument and the version in 1.23.0 does not require any argument.
Why it is not backardly compatible?
The flattening of arrays into veradic arguments is broken ... see below.

On go 1.19.3, 1.19.2 and 1.18.5 we have this issue which you can replicate using this sample test code

func test1(args ...string)              {}
func test2(name string, args ...string) {}

func testFail() {
	colours := []string{"orange", "green"}
        // this does not compile: Invalid use of '...', the corresponding parameter is non-variadic
	test1("Fred", colours...)
        // this compiles
	test2("Fred", colours...)
}

This change needs to be reverted or it needs to be 2.0.0 to keep the semantic versioning.

@onsi
Copy link
Owner

onsi commented Nov 9, 2022

Hey there @KevinJCross - sorry for the disruption. I just shipped 1.24.1 which restores backwards compatibility

@silvestre
Copy link

Hi @onsi, thank you very much for the super-quick fix! We were already able to update to the new version: cloudfoundry/app-autoscaler-release#1034

@KevinJCross
Copy link
Author

Thanks @onsi

@onsi
Copy link
Owner

onsi commented Nov 9, 2022

👍

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

No branches or pull requests

3 participants