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

Revert "[ci] enable coverage on merge job" #14804

Merged
merged 1 commit into from Dec 8, 2023

Conversation

dixler
Copy link
Contributor

@dixler dixler commented Dec 8, 2023

Fixes #14799
Reverts #14727

This PR reverts enabling coverage on the merge job.
prepare-release uses the binaries built during tests which were built with coverage.

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Dec 8, 2023

Changelog

[uncommitted] (2023-12-08)

Bug Fixes

  • [cli/display] Fixes displaying warning: GOCOVERDIR not set, no coverage data emitted
    #14804

Copy link
Member

@justinvp justinvp left a comment

Choose a reason for hiding this comment

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

LGTM, but needs a changelog entry about not emitting the warning.

As a follow-up, we should also have an integration test that runs a command like pulumi version and ensures we're not getting unexpected output to stderr/stdout.

@dixler dixler force-pushed the revert-14727-dixler-enable-coverage branch from b95bae0 to fc6317e Compare December 8, 2023 17:12
@dixler
Copy link
Contributor Author

dixler commented Dec 8, 2023

LGTM, but needs a changelog entry about not emitting the warning.

As a follow-up, we should also have an integration test that runs a command like pulumi version and ensures we're not getting unexpected output to stderr/stdout.

Just added this take a look.

Fixes #14799
This reverts commit b353d95.

This disables building binaries with coverage. The binaries are used in
a downstream prepare-release job and causes an error message:

`warning: GOCOVERDIR not set, no coverage data emitted`
@dixler dixler force-pushed the revert-14727-dixler-enable-coverage branch from fc6317e to 4daf352 Compare December 8, 2023 17:13
@dixler dixler enabled auto-merge December 8, 2023 17:15
@justinvp justinvp mentioned this pull request Dec 8, 2023
@dixler dixler added this pull request to the merge queue Dec 8, 2023
Merged via the queue into master with commit 061d10b Dec 8, 2023
45 checks passed
@dixler dixler deleted the revert-14727-dixler-enable-coverage branch December 8, 2023 19:01
github-merge-queue bot pushed a commit that referenced this pull request Dec 8, 2023
Will merge this after #14804
merges.
github-merge-queue bot pushed a commit that referenced this pull request Dec 12, 2023
…erage (#14806)

Fixes #14817 

As a follow-up to #14804, we need to re-enable code coverage without
releases being built with coverage. #14804 includes a test in the
release job to catch `pulumi` being released with coverage data.

Also fixes the test to check that the pulumi binary is not built with
coverage.

Background:
Coverage was disabled on merge jobs in order to fix
[#14799].
[#14716] removed nightly coverage workflows in favor of
collecting coverage on merges [#14727].
The binaries built with coverage were used in the downstream
prepare-release job causing a noticable bug [#14799].
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.

Pulumi CLI command return warning: "GOCOVERDIR not set, no coverage data emitted"
3 participants