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

Remove gotestfmt #1069

Merged
merged 4 commits into from
Sep 5, 2024
Merged

Remove gotestfmt #1069

merged 4 commits into from
Sep 5, 2024

Conversation

t0yv0
Copy link
Member

@t0yv0 t0yv0 commented Sep 5, 2024

Engineers noticing gotestfmt reporting test output in a confusing way, such as mis-attributing logs from one test to another or not reporting logs if a panic is encountered. This change removes this utility, switching to default Go test output. It also removes the -v flag so that the test failure output is minimally scoped to the failing tests and is easier to find. Defaults for the win.

@t0yv0 t0yv0 requested review from blampe and a team September 5, 2024 21:21
Copy link
Contributor

@guineveresaenger guineveresaenger left a comment

Choose a reason for hiding this comment

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

Let's see what this looks like!

@t0yv0 t0yv0 added this pull request to the merge queue Sep 5, 2024
Merged via the queue into master with commit c98af28 Sep 5, 2024
9 checks passed
@t0yv0 t0yv0 deleted the t0yv0/remove-gotestfmt branch September 5, 2024 23:02
@thomas11
Copy link
Contributor

thomas11 commented Sep 6, 2024

@t0yv0, @guineveresaenger Late to the party but we've had the same issues in azure-native and I had good initial results with tparse instead. Here's a draft PR; you can look at the test results. It's essentially just regular go test output but with a summary at the end.

t0yv0 added a commit that referenced this pull request Sep 6, 2024
@t0yv0 t0yv0 mentioned this pull request Sep 6, 2024
github-merge-queue bot pushed a commit that referenced this pull request Sep 6, 2024
Reverts #1069

I'll redo it without removing GoTestFmt install step to fix:

#1070
github-merge-queue bot pushed a commit that referenced this pull request Sep 9, 2024
Take 2 on removing gotestfmt. Since extension workflows still use it,
retain the installation stanza, but not actually use in tests.

pulumi/pulumi-azure-native#3404 is promising but
to be evaluated later I think?

Previously: #1069
#1071
thomas11 added a commit to pulumi/pulumi-azure-native that referenced this pull request Sep 27, 2024
Following ci-mgmt where it [was already
removed](pulumi/ci-mgmt#1069).
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.

5 participants