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

feat: Add report_type column to commit reports #287

Merged
merged 6 commits into from
Dec 6, 2023

Conversation

scott-codecov
Copy link
Contributor

Purpose/Motivation

To support test results and bundle analysis ingestion. This will allow us to reuse the reports_upload table for new types of ingest and process those uploads into reports_commitreport records for other types of per-commit reports.

Here's some initial work on how worker will handle this: codecov/worker#190

Links to relevant tickets

N/A

What does this PR do?

Adds a new report_type column to reports_commitreport. Existing rows will be null and null will be interpreted to mean "coverage" reports. Valid values for this column are "coverage", "test_results" and "bundle_analysis".

Existing queries for commit reports were updated to include only "coverage" reports. They can be updated in the future as needed to handle switching between the various report types.

@codecov-qa
Copy link

codecov-qa bot commented Dec 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b5ddf2d) 95.64% compared to head (5bfcbcb) 95.65%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #287   +/-   ##
=======================================
  Coverage   95.64%   95.65%           
=======================================
  Files         610      612    +2     
  Lines       15714    15734   +20     
=======================================
+ Hits        15030    15050   +20     
  Misses        684      684           
Flag Coverage Δ
unit 95.65% <100.00%> (+<0.01%) ⬆️
unit-latest-uploader 95.65% <100.00%> (+<0.01%) ⬆️

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

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

Copy link

codecov bot commented Dec 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a29cd70) 95.55% compared to head (bdafb98) 95.56%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main    #287     +/-   ##
=======================================
+ Coverage   95.55   95.56   +0.01     
=======================================
  Files        724     726      +2     
  Lines      16121   16141     +20     
=======================================
+ Hits       15404   15424     +20     
  Misses       717     717             
Flag Coverage Δ
unit 95.65% <100.00%> (+<0.01%) ⬆️
unit-latest-uploader 95.65% <100.00%> (+<0.01%) ⬆️

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

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

Copy link
Contributor

@JerrySentry JerrySentry left a comment

Choose a reason for hiding this comment

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

LGTM thank you!

  • Perhaps its better to have a report_type type as enum for this new column in the DB.
  • Is the report_uploads table currently fully compatible to support any type of reports?

@scott-codecov
Copy link
Contributor Author

scott-codecov commented Dec 4, 2023

@JerrySentry

Perhaps its better to have a report_type type as enum for this new column in the DB.

Yeah I suppose this will change so infrequently that it makes sense to encode this at the db level. Though I'm only seeing create type X as enum in our legacy migrations so maybe we've moved away from that approach.

Is the report_uploads table currently fully compatible to support any type of reports?

I think so - they're all very generic columns in the uploads table and are related more to a CI environment than anything Codecov specific. One thing to keep in mind is that it won't make sense to query for all uploads on a commit just by joining commitreport naively - there should always be a condition on the report_type column. (This was also previously the case with reports_commitreport.code though - it probably wouldn't make sense to show uploads across multiple codes)

Copy link

codecov-public-qa bot commented Dec 5, 2023

Codecov Report

Merging #287 (5bfcbcb) into main (b5ddf2d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #287   +/-   ##
=======================================
  Coverage   95.64%   95.65%           
=======================================
  Files         610      612    +2     
  Lines       15714    15734   +20     
=======================================
+ Hits        15030    15050   +20     
  Misses        684      684           
Flag Coverage Δ
unit 95.65% <100.00%> (+<0.01%) ⬆️
unit-latest-uploader 95.65% <100.00%> (+<0.01%) ⬆️

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

Files Coverage Δ
core/models.py 94.61% <100.00%> (ø)
graphql_api/actions/commits.py 100.00% <ø> (ø)
graphql_api/dataloader/commit.py 100.00% <100.00%> (ø)
reports/managers.py 100.00% <100.00%> (ø)
...eports/migrations/0011_commitreport_report_type.py 100.00% <100.00%> (ø)
reports/models.py 96.49% <100.00%> (+0.22%) ⬆️
services/comparison.py 99.33% <ø> (ø)
services/report.py 96.63% <ø> (ø)

Impacted file tree graph

@scott-codecov scott-codecov merged commit 6e841d6 into main Dec 6, 2023
16 of 17 checks passed
@scott-codecov scott-codecov deleted the scott/commit-report-type branch December 6, 2023 13:51
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.

3 participants