-
Notifications
You must be signed in to change notification settings - Fork 129
Reorganize integration tests #1120
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
Conversation
5d8661f to
8d3992a
Compare
683b4ce to
254f6ba
Compare
254f6ba to
b5d5452
Compare
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "go.testTags": "linux_and_mac", | |||
| "go.buildTags": "linux_and_mac" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without these tags, the Go LSP doesn't handle the integration test files (since most of them are explicitly tagged with this)
| @@ -1,82 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused file, couldn't find any references to it
| @@ -1,61 +0,0 @@ | |||
| format_version: 1.3.0 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused file too
| @@ -1,23 +0,0 @@ | |||
| //go:build linux_and_mac | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not deleted, just moved
Checklist
README.mdis updated with the changes (if needed)Version
Requires a MAJOR/MINOR/PATCH version update
Context
Integration tests at the moment are in a folder called
_testsin order to exclude them from the Go package when other packages import it as a dependency. This has two drawbacks, all because of the fact that we disable Go module features:Changes
The main magic trick is this
go.modfile:Additional changes:
go testcommand had a non-zero exit code.settings.jsonfor a nice default experienceInvestigation details
Verification that this PR doesn't delete or disable tests accidentally:
Reference build from
master: 101 integration testsPR build: 101 integration tests
Setting
-p 1to serially execute tests unfortunately regresses test run time from ~7 minutes to ~11 minutes, but we can easily shard tests in the future by the Go modules this PR creates.Decisions
Out of scope for this PR in order to keep the diff small:
$INTEGRATION_TEST_BINARY_PATH...it's going to be more complicated