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 test runner to Go 1.22 #113

Merged
merged 3 commits into from
Jul 30, 2024

Conversation

andrerfcsantos
Copy link
Member

@andrerfcsantos andrerfcsantos commented Jul 22, 2024

Summary

This PR updates the test runner to use Go 1.22.

Go 1.22 introduces some changes to the test output, and the changes in the code of this PR accommodate for that.

Details

The main difference in the test output is that previously when a test would fail due to an error in compilation of the test, the go test command would produce a textual output without any json lines, despite the tests being run with the --json flag. Now the go test command respects the --json flag, even when the test doesn't compile, and some lines of the output are json lines.

Go 1.22 vs Go 1.21 go test --json output differences
$ go1.22.5 test -v --json
# gigasecond [gigasecond.test]
./missing_func_test.go:39:11: undefined: AddGigasecond
./missing_func_test.go:72:11: undefined: AddGigasecond
{"Time":"2024-07-22T10:30:50.345109+01:00","Action":"start","Package":"gigasecond"}
{"Time":"2024-07-22T10:30:50.34516+01:00","Action":"output","Package":"gigasecond","Output":"FAIL\tgigasecond [build failed]\n"}
{"Time":"2024-07-22T10:30:50.345165+01:00","Action":"fail","Package":"gigasecond","Elapsed":0}
$ go1.21.12 test -v --json
# gigasecond [gigasecond.test]
./missing_func_test.go:39:11: undefined: AddGigasecond
./missing_func_test.go:72:11: undefined: AddGigasecond
FAIL	gigasecond [build failed]

Previously, the test runner was ignoring any json lines in the test output when the tests failed to compile. Now it doesn't ignore them, and includes some information of them in the test report.

Complete list of changes

  • Updated the Go version of the test runner to 1.22.5 (latest Go 1.22 release at the time of this PR), along with the base alpine image.

    From running the tests in the repo and also running the images manually with some examples, no regressions were found. The test outputs are the expected ones, and no significant differences found in the time the tests took to run.

  • Updated the dependency stretchr/testify (v1.8.2 -> v1.9.0)

    There was no particular reason for this, other than just trying to keep the dependencies up-to-date and avoid big bumps in versions later.

  • Updated golangci-lint

    CI was failing with some errors due to the version of golangci-lint not being up-to-date. The github action was updated as well as the version of golangci-lint the action uses.

  • Separation of the logic of parsing the test output and processing it into two separate functions: parseTestOutput and processTestResults.

    Previously this logic was mixed together into a single function, and parsing the test output (including looking for json lines) would only apply if the tests compiled and ran to completion. Since we now need this logic even if the tests don't run, made sense to make these 2 be separate steps.

  • Changed the expectations for when the tests don't compile / run to completion, the package level messages now appear first.

    Previously the package level messages were appearing somewhere in the middle of the output. To keep the expectation as is, we would have to implement logic to insert these package level messages in the middle of the failed messages, which seems messy. I think changing the expectation slightly is fine in the case - the message contains the same information as before, just ordered differently.

@andrerfcsantos andrerfcsantos requested a review from a team as a code owner July 22, 2024 11:44
Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look like excellent improvements!

@andrerfcsantos andrerfcsantos merged commit b891fa8 into exercism:main Jul 30, 2024
5 checks passed
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