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

ci: get list of tests to run and skip from codecov ats #12

Closed
wants to merge 20 commits into from

Conversation

giovanni-guidini
Copy link
Owner

Mostly these changes alter the codecov_ats workflow to generate 2 outputs:

  • ATS_TESTS_TO_RUN - list of tests (and options to pytest) that codecov thinks you should run
  • ATS_TESTS_TO_SKIP - list of tests (and options to pytest) that codecov thinks you should skip

There are some config changes to codecov.yml:

  • The yaml validator was accusing coverage.status.project and coverage.status.patch to be invalid because of the "default" key.
  • Adding a "smart-tests" flag that would be used with ATS (if the tests are run)

Another notable change is the fetch-depth when checking out the code from 0 to 2. I changed the BASE_COMMIT to be the parent commit, so 2 should suffice.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Mostly these changes alter the codecov_ats workflow to generate 2 outputs:
* ATS_TESTS_TO_RUN - list of tests (and options to pytest) that codecov thinks you should run
* ATS_TESTS_TO_SKIP - list of tests (and options to pytest) that codecov thinks you should skip

There are some config changes to `codecov.yml`:
* The yaml validator was accusing coverage.status.project and coverage.status.patch to be invalid because of the "default" key.
* Adding a "smart-tests" flag that would be used with ATS (if the tests are run)

Another notable change is the `fetch-depth` when checking out the code from 0 to 2.
I changed the BASE_COMMIT to be the parent commit, so 2 should suffice.
From https://github.com/orgs/community/discussions/46992#discussioncomment-4961541

This should allow us to populate the appropriate env vars even if the label-analysis step fails.
Which is not the case currently (see https://github.com/giovanni-guidini/sentry/actions/runs/6409333248/job/17400305852)
These changes add a special upload to codecov step after the backend tests.
It uses a JSON report that is generated with test execution info (`.artifacts/python.coverage.json`),
and uploads with a `smart-tests` flag.

Notice that this `smart-tests` flag doesn't have any statuses activated for it.
@codecov
Copy link

codecov bot commented Oct 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f421fee) 86.71% compared to head (d82deac) 81.03%.

❗ Current head d82deac differs from pull request most recent head 230009f. Consider uploading reports for the commit 230009f to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           master      #12       +/-   ##
===========================================
- Coverage   86.71%   81.03%    -5.68%     
===========================================
  Files        2632     4380     +1748     
  Lines      157909   321029   +163120     
  Branches    27286    35538     +8252     
===========================================
+ Hits       136924   260139   +123215     
- Misses      15677    55035    +39358     
- Partials     5308     5855      +547     
Flag Coverage Δ
backend 80.93% <ø> (-5.62%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2664 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Using https://github.com/giovanni-guidini/components-demo as a POC for these config changes.

- JSON report needs to be created separated to have the contexts information
- We have to clean up XML report before uploading the ATS one
Update pros-processing to better debug values and display how they can be used with pytest.

The fallbacks are definetelly working. Check [this example in the POC](https://github.com/giovanni-guidini/components-demo/actions/runs/6537092148/job/17750193394)
The actual working path has issues around the doublequoting of test names. Check [this example in the POC](https://github.com/giovanni-guidini/components-demo/actions/runs/6537095654/job/17750566321?pr=21)

Instead of changing the post-processing to replace doublequotes (which is a possibility),
it might be worth it to wait until a new CLI release is made.
It will include codecov/codecov-cli@0e73c77 that fixes this issue
(and makes the output cleaner and easier to parse)
Changes introduced to `backend.yml` and the `test-python-ci` target
are being removed.

This is because there was evidence that using `--cov-context=test` in pytest
would incur a 22% (median) increase in CI time. That was deemed unacceptable.

However Codecov's Automated Test Selection still needs this data to be able
to correlate test execution and coverage information with the test that
generated that coverage.

The agreed upon solution will be a periodic job to collect coverage info in
the master branch and a cheaper job to carryforward coverage reports for
all new commits in the master branch.

These changes will be made in a separate PR.
giovanni-guidini and others added 7 commits October 19, 2023 10:42
This version of the Codecov CLI has cleaner output for label-analysis so
post-processing is more straight forward.

Also improving the post-processing of failed label-analysis as suggested by @bukzor-sentryio
Fixing the post processing of label analysis failure based on POC runs
(see https://github.com/giovanni-guidini/components-demo/actions/runs/6579967512/job/17876943612 for the end result)

The strange syntax came from github/docs#21529

Also letting label-analysis step fail (_after_ the post processing)
These changes are better suited for a different PR,
that doesn't yet exist (😅) but will very shortly.

This is because it has been decided that the necessary inputs for
Automated Test Selection will be collected in different workflows.

The changes here (in codecov.yml) are releated to that collection
Given how we will collect the necessary inputs for ATS
(in a periodic workflow in the master branch)
it makes more sense to use commits in the master branch as the BASE
for ATS.

The changes here change the BASE picking to those commits
(i.e. the base of a feature branch)
Also adding comment that `--raw-output0` doesn't work with `jq` in GHActions.
I tested it in https://github.com/giovanni-guidini/components-demo/actions/runs/6591431068/job/17910020151
The error message is `jq: Unknown option --raw-output0`
@giovanni-guidini giovanni-guidini force-pushed the master branch 4 times, most recently from 49033ea to fe0b1cb Compare October 31, 2023 18:03
@giovanni-guidini giovanni-guidini deleted the gio/ats-update branch November 7, 2023 13:08
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.

1 participant