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.

Sign up for GitHub

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

Jump to bottom

ci: Add coverage tracking #469

Merged
merged 4 commits into from
Sep 4, 2023
Merged

ci: Add coverage tracking #469

merged 4 commits into from
Sep 4, 2023

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Aug 1, 2023 •
edited
Loading

Adds a make test_cover target to the Makefile
which builds pulumi-language-yaml with coverage instrumentation
and runs all tests with coverage tracking.

For CI, adds an coverage option that, when enabled,
will run make test_cover instead of make test,
and upload the results to codecov.
This option is only enabled for tests invoked for PRs
and by the /run-acceptance-tests command.

Additionally, similarly to pulumi/pulumi#13334,
this adds a workflow that runs tests on master with coverage tracking
every 12 hours.

Resolves #464

@abhinav abhinav added the impact/no-changelog-required This issue doesn't require a CHANGELOG update label Aug 1, 2023
@abhinav abhinav changed the base branch from main to abhinav/go120 August 1, 2023 18:11
@abhinav abhinav mentioned this pull request Aug 1, 2023
Merged
@abhinav Graphite App
Copy link
Contributor Author

abhinav commented Aug 1, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@abhinav abhinav mentioned this pull request Aug 1, 2023
Merged
@abhinav abhinav mentioned this pull request Aug 1, 2023
Merged
@abhinav abhinav mentioned this pull request Aug 1, 2023
Merged
Base automatically changed from abhinav/go120 to main August 1, 2023 19:44
Frassle
Frassle previously approved these changes Aug 1, 2023
View reviewed changes
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.

Looks sane, I imagine we're gonna end up with some cross repo duplication here but feels like we can come back to that when stable.

@abhinav abhinav changed the base branch from main to abhinav/pulumi-up August 1, 2023 20:43
This was referenced Aug 1, 2023
Merged
Merged
Base automatically changed from abhinav/pulumi-up to main August 1, 2023 22:13
@abhinav abhinav force-pushed the abhinav/coverage branch 4 times, most recently from 59cef54 to c024792 Compare August 7, 2023 16:11
@abhinav
Copy link
Contributor Author

abhinav commented Aug 8, 2023

From local experimentation on master, it seems that this test is just really long-running. It takes 7+ minutes to delete the aws:ecs:Service for some reason, which eats up the bulk of the test runtime.

@abhinav
Copy link
Contributor Author

abhinav commented Aug 8, 2023

Sounds like some AWS resources taking long to delete is a known thing.
That explains the 30m timeout on make test.

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@dfcddb9). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #469   +/-   ##
=======================================
  Coverage        ?   61.90%           
=======================================
  Files           ?       30           
  Lines           ?     7533           
  Branches        ?        0           
=======================================
  Hits            ?     4663           
  Misses          ?     2485           
  Partials        ?      385

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@abhinav abhinav marked this pull request as draft August 9, 2023 22:52
@abhinav abhinav mentioned this pull request Aug 9, 2023
Closed
@abhinav abhinav changed the base branch from main to abhinav/sigint August 28, 2023 21:47
@abhinav abhinav mentioned this pull request Aug 28, 2023
Merged
@abhinav
Copy link
Contributor Author

abhinav commented Aug 28, 2023

Okay, so this turned into a yak shave.

#464 (comment) has an update from 3 weeks ago on the tooling side. On top of that, even with the tooling, there was another problem: we SIGKILLed all plugins so we couldn't get plugin coverage data anyway.

However, with the changes I've made to pu/pu recently, namely the following, plugins can now be terminated gracefully.

I've stacked this PR on top of #501. It also needs a version of pu/pu with the above two PRs merged into it, after which binaries are able to produce meaningful coverage data.

@abhinav abhinav dismissed Frassle’s stale review August 28, 2023 22:03

The PR has changed quite significantly since initial review.

@abhinav abhinav force-pushed the abhinav/coverage branch 6 times, most recently from 47cbb10 to 54079d5 Compare August 28, 2023 23:19
Base automatically changed from abhinav/sigint to main August 29, 2023 15:54
@abhinav abhinav force-pushed the abhinav/coverage branch 2 times, most recently from 04536a9 to e5cd816 Compare August 29, 2023 16:18
@abhinav
Copy link
Contributor Author

abhinav commented Aug 29, 2023

It works!

image

The graceful shutdown work in pu/pu recently allows pulumi-language-yaml to write the coverage data.

@abhinav @justinvp
ci: Add coverage tracking
106bc2c 
Adds a `make test_cover` target to the Makefile
which builds pulumi-language-yaml with coverage instrumentation
and runs all tests with coverage tracking.

Note that right now, there are no tests that invoke pulumi-language-yaml
so integration test data is empty.

For CI, adds an coverage option that, when enabled,
will run `make test_cover` instead of `make test`,
and upload the results to codecov.
This option is only enabled for tests invoked for PRs
and by the `/run-acceptance-tests` command.

Additionally, similarly to pulumi/pulumi#13334,
this adds a workflow that runs tests on master with coverage tracking
every 12 hours.

Resolves #464
@abhinav @justinvp
Add gocov tool
db969e1 
Adds an experimental tool that runs `go test -c` and then runs the tests
for each package separately.

This will provide combined coverage data for unit and integration tests.
@abhinav @justinvp
gocv: Support test2json
c59370b 
Adds a flag to use test2json to run the test binary,
enabling compatibility with gotestsum.
@abhinav @justinvp
gocov: Add -race flag
91a783f
@justinvp justinvp marked this pull request as ready for review September 4, 2023 18:08
justinvp
justinvp approved these changes Sep 4, 2023
View reviewed changes
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

@justinvp justinvp merged commit 0db26f7 into main Sep 4, 2023
7 checks passed
@justinvp justinvp deleted the abhinav/coverage branch September 4, 2023 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@justinvp justinvp justinvp approved these changes

@Frassle Frassle Frassle left review comments

Assignees
No one assigned
Labels
impact/no-changelog-required This issue doesn't require a CHANGELOG update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add coverage data to codecov
4 participants
@abhinav @codecov-commenter @justinvp @Frassle