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

Apply goimports before checking make sizegen #8828

Merged
merged 11 commits into from
Sep 17, 2021

Conversation

frouioui
Copy link
Member

@frouioui frouioui commented Sep 16, 2021

Description

After having issues with check_make_sizegen due to a difference between the source file (on which goimports was applied) and the file generated by Jennifer in the import statement, CI builds started failing in this pull request with the following error:

--- FAIL: TestFullGeneration (1.66s)
    sizegen_test.go:31: 
        	Error Trace:	sizegen_test.go:31
        	Error:      	Should be empty, but was ['/home/runner/work/vitess/vitess/go/tools/sizegen/integration/cached_size.go' has changed]
        	Test:       	TestFullGeneration

To alleviate this issue, this pull request changes how the TestFullGeneration test compares the local and generated files. After Jennifer has generated the file, we write its content to a temporary file, execute goimports on it, and finally, we run the comparison between the local and generated files. This ensures that we compare local files against goimports'ed files. And to make sure our local files are always goimports'ed, this pull request adds a new workflow (check_imports) that fails if a file is not goimports'ed.

Adding this new workflow requires the installation of goimports, to do that, it was necessary to upgrade setup-go action from v1 to v2 as explained in this issue: actions/setup-go#27.

Related Issue(s)

Checklist

  • Tests were added or are not required
  • Documentation was added or is not required

@frouioui frouioui force-pushed the fix-make-sizegen branch 2 times, most recently from 3ad3ca1 to 9a57c9b Compare September 16, 2021 10:57
@frouioui frouioui marked this pull request as ready for review September 16, 2021 13:59
@systay systay requested review from vmg and removed request for rohit-nayak-ps September 16, 2021 14:45
Copy link
Member

@GuptaManan100 GuptaManan100 left a comment

Choose a reason for hiding this comment

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

Everything else LGTM! 🍰

.github/workflows/check_imports.yml Outdated Show resolved Hide resolved
@vmg vmg merged commit 51c6c54 into vitessio:main Sep 17, 2021
@vmg vmg deleted the fix-make-sizegen branch September 17, 2021 08:44
@frouioui frouioui mentioned this pull request Sep 22, 2021
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants