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

truncate debug log output in conformance tests #16117

Merged
merged 2 commits into from May 6, 2024
Merged

Conversation

tgummerer
Copy link
Collaborator

This debug log output can get way too long, and GitHub Actions logs can't even display it anymore when there are test failures. Cut each individual message short, with an escape hatch in case someone wants to run with the full output locally.

I had a look at the test failure in #16116 and it seems like it's still having the same issue. Looking again at the test output it's actually this t.Log that includes the registered resource outputs, that contains the large amounts of data. I arbitrarily decided to make the cutoff 100 chars, but that could be adjusted.

This debug log output can get way too long, and GitHub Actions logs
can't even display it anymore when there are test failures.  Cut each
individual message short, with an escape hatch in case someone wants
to run with the full output locally.
@tgummerer tgummerer requested a review from a team as a code owner May 3, 2024 14:41
@tgummerer tgummerer added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label May 3, 2024
@pulumi-bot
Copy link
Contributor

Changelog

[uncommitted] (2024-05-03)

@tgummerer
Copy link
Collaborator Author

Copy link
Member

@Frassle Frassle left a comment

Choose a reason for hiding this comment

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

Nice find. I think we might want #16116 as well if stdout/stderr logs are big.

@tgummerer tgummerer added this pull request to the merge queue May 6, 2024
@tgummerer
Copy link
Collaborator Author

tgummerer commented May 6, 2024

Nice find. I think we might want #16116 as well if stdout/stderr logs are big.

Yeah I don't mind having that merged as well. We shouldn't need that output for successful tests in any case.

Merged via the queue into master with commit 9b4c9dc May 6, 2024
49 checks passed
@tgummerer tgummerer deleted the tg/shorten-output branch May 6, 2024 09:30
github-merge-queue bot pushed a commit that referenced this pull request May 6, 2024
Sometimes we show hundreds of repeated messages in the logs, e.g.
"waiting for quiescence; 3 outputs outstanding". This just makes the
logs harder to read. Since we need to wait for the test to finish before
any of this is shown anyway, only log each message once and show a
little blurb about how often the message has been repeated.

(Note that for simplicity if the last message is repeated we don't show
the same blurb, but that should be okay since the last messages we're
logging are always different.)

Based on #16117 where I noticed
this, and I didn't want to rebase it to avoid conflicts.
tgummerer added a commit that referenced this pull request May 16, 2024
We initially introduced this limit for log messages in
#16117.  However I've since
realized that these log messages also include useful
stacktraces (when the program being tested throws an exception), which
are a bit longer.

The intention here is really to avoid *really* long messages, so
increase the maximum message size a little to still avoid those, but
allow us to see more of the stacktrace that should hopefully be useful
for debugging.

The number is mostly still based on a guesstimate of what's useful,
but it's easy to tweak if we need to tweak it further.
github-merge-queue bot pushed a commit that referenced this pull request May 16, 2024
…6213)

We initially introduced this limit for log messages in
#16117. However I've since realized
that these log messages also include useful stacktraces (when the
program being tested throws an exception), which are a bit longer.

The intention here is really to avoid *really* long messages, so
increase the maximum message size a little to still avoid those, but
allow us to see more of the stacktrace that should hopefully be useful
for debugging.

The number is mostly still based on a guesstimate of what's useful, but
it's easy to tweak if we need to tweak it further.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants