-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Don't run tests in verbose mode in CI #444
Conversation
@@ -28,7 +28,7 @@ jobs: | |||
go-version: '1.21' | |||
|
|||
- name: Test | |||
run: go test -v ./... | |||
run: go test ./... |
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.
I don't think that you have to dig through lots of output should be that big of a deal (I don't think it's that much either). Granted Go makes it hard to see when test output begins, but we put in work to separate the pieces. Just because the action you're using as an impetus for this PR didn't require verbose output to see the problem doesn't mean other people's test failures don't. I don't think the logs are that egregious.
I think we should err on the side of more information in this case (it's very reasonable for CLI tests to show their output even on success).
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.
Update: To clarify, since verbose is automatically applied on the test that fails, I can be ok with this change. I would like to hear others' opinion here real quick before approving though.
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.
I'm only proposing this for that exact reason: failing tests will always be dumped verbosely :)
When you run in verbose mode you need to dig through _all_ test outputs and logging to find even a single test failure. In https://github.com/temporalio/cli/actions/runs/7877883209/job/21494958729?pr=443 we have 1000 lines of output and only 20 lines were relevant to the actual failing test.
270d55b
to
f9aabb0
Compare
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.
Looks like nobody came with another opinion, marking approved
What was changed
CI will now run tests without
-v
.Why?
When you run in verbose mode you need to dig through all test outputs and logging to find even a single test failure. In https://github.com/temporalio/cli/actions/runs/7877883209/job/21494958729?pr=443 we have 1000 lines of output and only 20 lines were relevant to the actual failing test.