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: don't skip acceptance tests when targeted with -run #1630

Merged
merged 2 commits into from
Feb 17, 2025

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Feb 17, 2025

This is a neat little trick I thought of while cleaning up the snapshots - all its saving us from is having to stick TEST_ACCEPTANCE=true in front of our go test calls, but I think it's nice to have the test suite not require that when you're running a specific acceptance test.

Credit to go-snaps for proving it must be possible, since this is what it does to determine if a snapshot is obsolete or that the tests have just not been run.

I've gone with the slightly awkward name isThisTestRunTarget as I felt it was less ambiguous than isThisTestRunning and similar, since yes the test must be running if the function is being called... 😅

@G-Rath G-Rath force-pushed the test/skip-unless-called branch from aeab4f1 to 7a4d385 Compare February 17, 2025 19:42
@G-Rath G-Rath force-pushed the test/skip-unless-called branch from 7a4d385 to edf623e Compare February 17, 2025 19:43
func isThisTestRunTarget(t *testing.T) bool {
t.Helper()

runOnly := flag.Lookup("test.run").Value.String()
Copy link
Contributor

Choose a reason for hiding this comment

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

the flag is run or test.run?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

test.run, as that's actually what is used under the hood I believe because really things are being compiled into a binary, so this prefix helps avoid clashes with tests & binaries that include flags.

From the docs:

Each of these flags is also recognized with an optional 'test.' prefix,
as in -test.v. When invoking the generated test binary (the result of
'go test -c') directly, however, the prefix is mandatory.

Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

Neat! Hopefully this means I can debug an acceptance test in vscode without having to comment out the "skip if not acceptance test" function now.

@another-rex another-rex enabled auto-merge (squash) February 17, 2025 23:47
@another-rex another-rex merged commit 7d82c34 into google:main Feb 17, 2025
13 checks passed
@another-rex another-rex deleted the test/skip-unless-called branch February 17, 2025 23:53
@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2025

Codecov Report

Attention: Patch coverage is 12.50000% with 7 lines in your changes missing coverage. Please review.

Project coverage is 68.75%. Comparing base (3bd1565) to head (860bbc8).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
internal/testutility/utility.go 12.50% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1630      +/-   ##
==========================================
- Coverage   68.77%   68.75%   -0.03%     
==========================================
  Files         199      199              
  Lines       18949    18956       +7     
==========================================
+ Hits        13032    13033       +1     
- Misses       5221     5227       +6     
  Partials      696      696              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

4 participants