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

test: update matrix for Go 1.20 #1130

Merged
merged 2 commits into from
Feb 9, 2023
Merged

test: update matrix for Go 1.20 #1130

merged 2 commits into from
Feb 9, 2023

Conversation

blgm
Copy link
Collaborator

@blgm blgm commented Feb 1, 2023

No description provided.

@onsi
Copy link
Owner

onsi commented Feb 1, 2023

hmm looks like that might be a real issue - i can poke at it later this week

@blgm
Copy link
Collaborator Author

blgm commented Feb 9, 2023

@onsi I've pushed a change to the test to "fix" the issue. It was relying on getting a very specific coverage number (71.4%) and that's now changed in Go 1.20 (to 80%). I guess increased coverage is good, and I'm highly sceptical of relying on coverage numbers other than 100% as an optimiser can alter the number of statements and change the result.

The changed code will accept either 71.4% or 80%. I'm a little uncomfortable with such an unsophisticated fix, but we are testing integration with Go coverage rather than how the numbers are calculated. I'll hold off merging in case you have thoughts.

@onsi
Copy link
Owner

onsi commented Feb 9, 2023

hey @blgm all your caveats make sense and that fix sounds good to me. thanks for fixing it!

@blgm blgm merged commit 4890a62 into master Feb 9, 2023
@blgm blgm deleted the go1.20 branch February 9, 2023 15:44
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.

2 participants