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

chore: use -coverpkg for extended code coverage reports #1122

Closed
wants to merge 8 commits into from

Conversation

moul
Copy link
Member

@moul moul commented Sep 14, 2023

As discussed during the review meeting.

h/t @thehowl.

Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
@moul moul self-assigned this Sep 14, 2023
@moul moul changed the title chore: use -coverpkg for extended code coverage reporst chore: use -coverpkg for extended code coverage reports Sep 14, 2023
@moul moul marked this pull request as ready for review September 14, 2023 17:56
@moul moul requested a review from a team as a code owner September 14, 2023 17:56
@thehowl
Copy link
Member

thehowl commented Sep 14, 2023

Some weird results:

image

Any idea why some tests seem to show "decreasing coverage"?

also/ it looks like the YAML from the last PR is invalid: https://app.codecov.io/gh/gnolang/gno/commit/f49e9d03077f7661619791e52a5112d70ea019d9

@moul I saw in the last codecov PR you were complaining about not being able to validate the YAML until merged into master. They actually support validation, but you must do it through a HTTP request to their server:

curl --data-binary @/dev/stdin https://codecov.io/validate << EOF
codecov:
  notify:
    wait_for_ci: false

ignore:
  - misc

comment:
  require_changes: true
[...]
EOF
Error at ['codecov', 'require_changes']:
unknown field

@moul
Copy link
Member Author

moul commented Sep 16, 2023

Any idea why some tests seem to show "decreasing coverage"?

Maybe the various files are not merged, so the last one only is taken into account for each referenced file?

also/ it looks like the YAML from the last PR is invalid: app.codecov.io/gh/gnolang/gno/commit/f49e9d03077f7661619791e52a5112d70ea019d9

Thanks, fixed with #1137.

@moul I saw in the last codecov PR you were complaining about not being able to validate the YAML until merged into master. They actually support validation, but you must do it through a HTTP request to their server:

Thanks, but I was not talking about validation, but checking the impact of the change itself.

moul added a commit that referenced this pull request Sep 16, 2023
See #1122 (comment)

```console
❯ cat codecov.yml | curl --data-binary @/dev/stdin https://codecov.io/validate
Valid!

{
  "codecov": {
    "require_ci_to_pass": false,
...
```

Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
@moul moul added this to the 🌟 main.gno.land (wanted) milestone Sep 16, 2023
@codecov
Copy link

codecov bot commented Sep 16, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (ce258b1) 47.31% compared to head (98bf5c1) 66.57%.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1122       +/-   ##
===========================================
+ Coverage   47.31%   66.57%   +19.25%     
===========================================
  Files         367      418       +51     
  Lines       62118    63476     +1358     
===========================================
+ Hits        29394    42260    +12866     
+ Misses      30325    18367    -11958     
- Partials     2399     2849      +450     

see 251 files with indirect coverage changes

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

@moul
Copy link
Member Author

moul commented Sep 16, 2023

See #1139

@thehowl
Copy link
Member

thehowl commented Oct 5, 2023

I suspect the pkg1 and pkg2 tests to be failing because enabling coverage reporting on so many packages probably slows them down too much (adding the flag rewrites the code in order to count the lines covered). Obviously we have an issue of the two tests being too slow in general, but I think we might still improve our coverage data with what we actually test by having -coverpkg on gnovm/pkg/gnolang for all tests of package gnovm/tests. (Additionally, the same would probably need to be done for txtar, but we can add that later)

@thehowl
Copy link
Member

thehowl commented Mar 26, 2024

#1220 (comment)

@thehowl thehowl closed this Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Status: 🌟 Wanted for Launch
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants