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

update go to 1.22.3 #4184

Merged
merged 4 commits into from
Jun 12, 2024
Merged

update go to 1.22.3 #4184

merged 4 commits into from
Jun 12, 2024

Conversation

singholt
Copy link
Contributor

@singholt singholt commented May 21, 2024

Summary

This PR updates the go version we use in this repo to 1.22.3.

Implementation details

This PR has 4 commits:

  1. The first commit updates the GO_VERSION and GO_VERSION_WINDOWS files.

  2. The second commit addresses the failing go mod checks.
    I ran go mod tidy and go mod vendor to fix that.

  3. The third commit addresses the failing static checks.
    The specific static check version we were using had compatibility issues with the new go version: Ref. We now consume a compatible static check version.

  4. The last commit addresses code coverage failures.

  • We require that all our packages in this repository have at-least 60% code coverage, ref. With the go 1.22 update in this PR, our code coverage dropped to ~35%, from ~77%, with no new code added. There's also multiple upstream issues related to go code coverage calculations, example.
  • In 2022, there was a proposal to Go, to extend code coverage functionality to Go applications, not just code packages. As part of implementing this, there was also some work done on re-designing the code coverage. Part of this redesign, a feature-flag was added to let customers opt-out of this redesign in case they encounter issues.
  • Hence, this commit sets the feature flag to opt out of the coverage redesign. This helps us mitigate the issue and unblock our go upgrade. It is important to note that this flag will eventually be removed in a later go version. Ref.

Testing

New tests cover the changes: no

Description for the changelog

Does this PR include breaking model changes? If so, Have you added transformation functions?

N/A

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@singholt singholt force-pushed the dev branch 13 times, most recently from 194b4e2 to 8b60a54 Compare May 22, 2024 17:47
@singholt singholt force-pushed the dev branch 2 times, most recently from 8b60a54 to 67a625c Compare June 5, 2024 22:18
@singholt singholt changed the title [wip] update go -> 1.22.3 update go to 1.22.3 Jun 5, 2024
@singholt singholt marked this pull request as ready for review June 5, 2024 23:01
@singholt singholt requested a review from a team as a code owner June 5, 2024 23:01
@sparrc
Copy link
Contributor

sparrc commented Jun 6, 2024

With the go 1.22 update in this PR, our code coverage dropped to ~35%, from ~77%, with no new code added.

LGTM but one Q: Is the new coverage percent accurate at all? Do we know what our "actual" code coverage is?

@singholt
Copy link
Contributor Author

With the go 1.22 update in this PR, our code coverage dropped to ~35%, from ~77%, with no new code added.

LGTM but one Q: Is the new coverage percent accurate at all? Do we know what our "actual" code coverage is?

Good question, I investigated this a bit more. The issue here is with the new go version, its treating folders like ./agent as having 0.0% coverage since there are no test files directly in the folder - all test files are within subfolders like agent/acs/....

Previously directories where there are no test files were being marked as "[no test files]" instead of the 0% coverage.

I compared the test coverage before and after the GOEXPERIMENT=nocoverageredesign flag: diff (left is without the flag, right is with the flag)

I also compared code coverage for this PR against the code coverage on the last merged PR: diff. The coverage percentage for each directory where there are test files is consistent as before.

@singholt singholt merged commit dcfc524 into aws:dev Jun 12, 2024
40 checks passed
@prateekchaudhry prateekchaudhry mentioned this pull request Jun 13, 2024
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.

5 participants