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: tell codecov to wait_for_ci to avoid flappy reports #1160

Merged
merged 12 commits into from
Sep 22, 2023

Conversation

moul
Copy link
Member

@moul moul commented Sep 21, 2023

I've conducted tests on my fork, which you can find at https://github.com/moul/gno.

The 'if' condition draws inspiration from @ajnavarro's contributions, so a shoutout to him for that.

Additionally, @ajnavarro pointed out gaps in our testing for certain packages. We need to strategize on expanding our test coverage to address these overlooked areas.

I suggest waiting for his alternative work before considering merging mine.

@moul moul requested a review from a team as a code owner September 21, 2023 12:50
@codecov
Copy link

codecov bot commented Sep 21, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.05% ⚠️

Comparison is base (61f3ada) 47.08% compared to head (968c04c) 47.03%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1160      +/-   ##
==========================================
- Coverage   47.08%   47.03%   -0.05%     
==========================================
  Files         365      365              
  Lines       61156    61156              
==========================================
- Hits        28793    28765      -28     
- Misses      30011    30039      +28     
  Partials     2352     2352              
Flag Coverage Δ
gno.land-_test.gnokey 0.00% <ø> (ø)
gno.land-_test.gnoland 88.14% <ø> (ø)
gno.land-_test.pkgs 27.88% <ø> (ø)
gnovm-_test.cmd 45.89% <ø> (ø)
gnovm-_test.gnolang.native 63.09% <ø> (ø)
gnovm-_test.gnolang.other 16.63% <ø> (ø)
gnovm-_test.gnolang.pkg0 17.98% <ø> (ø)
gnovm-_test.gnolang.pkg1 8.21% <ø> (ø)
gnovm-_test.gnolang.pkg2 9.87% <ø> (ø)
gnovm-_test.gnolang.realm 41.68% <ø> (ø)
gnovm-_test.gnolang.stdlibs 53.53% <ø> (ø)
gnovm-_test.pkg 25.96% <ø> (ø)
tm2-_test.flappy ∅ <ø> (?)
tm2-_test.pkg.amino 58.32% <ø> (ø)
tm2-_test.pkg.bft 63.69% <ø> (-0.24%) ⬇️
tm2-_test.pkg.others 59.23% <ø> (ø)

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

see 5 files with indirect coverage changes

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

Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
Signed-off-by: moul <94029+moul@users.noreply.github.com>
@github-actions github-actions bot added the 📦 🌐 tendermint v2 Issues or PRs tm2 related label Sep 21, 2023
@@ -1,7 +1,7 @@
codecov:
require_ci_to_pass: false
notify:
wait_for_ci: false
wait_for_ci: true
Copy link
Member Author

Choose a reason for hiding this comment

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

this is the trick; making codecov wait for all jobs to finish before generating a report.

project:
default:
target: auto
threshold: 0.5%
Copy link
Member Author

Choose a reason for hiding this comment

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

i think this one should stay low like 0.5, or even smaller.

patch:
default:
target: auto
threshold: 1%
Copy link
Member Author

Choose a reason for hiding this comment

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

while i think this one could potentially be increased, because I think it works per file, and sometimes, adding a unique line may be seen as 5% change.

Signed-off-by: moul <94029+moul@users.noreply.github.com>
@@ -20,7 +20,7 @@ jobs:
strategy:
fail-fast: false
matrix:
go-version:
goversion:
Copy link
Member Author

Choose a reason for hiding this comment

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

removing the dash to allow using the variable if if conditions

- "1.20.x"
- "1.21.x"
runs-on: ubuntu-latest
timeout-minutes: 30
steps:
- uses: actions/checkout@v4
Copy link
Member Author

Choose a reason for hiding this comment

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

moving the actions/checkout before setup-go, because setup-go is trying to cache dependencies based on go.mod and go.sum

- name: test
working-directory: gno.land
run: |
export GOPATH=$HOME/go
export GOTEST_FLAGS="-v -p 1 -timeout=30m -coverprofile=coverage.out -covermode=atomic"
make ${{ matrix.args }}
- if: runner.os == 'Linux'
- if: ${{ runner.os == 'Linux' && matrix.goversion == '1.21.x' }}
Copy link
Member Author

Choose a reason for hiding this comment

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

to prevent inconsistent results, let's only upload the coverage of the latest supported version, while still unit testing with the previous version.

uses: codecov/codecov-action@v3
with:
token: ${{ secrets.CODECOV_TOKEN }}
name: gno.land
flags: gno.land-${{matrix.args}}
files: ./gno.land/coverage.out
#fail_ci_if_error: ${{ github.repository == 'gnolang/gno' }}
fail_ci_if_error: false # temporarily
fail_ci_if_error: ${{ github.repository == 'gnolang/gno' }}
Copy link
Member Author

Choose a reason for hiding this comment

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

i think this is what generated failed CI because of codecov 502 errors; would be nice to setup an auto-retry mechanism

Signed-off-by: moul <94029+moul@users.noreply.github.com>
@github-actions github-actions bot removed the 📦 🌐 tendermint v2 Issues or PRs tm2 related label Sep 21, 2023
@moul moul added the 🐞 bug Something isn't working label Sep 21, 2023
- name: test
working-directory: gnovm
run: |
export GOPATH=$HOME/go
export GOTEST_FLAGS="-v -p 1 -timeout=30m -coverprofile=coverage.out -covermode=atomic"
make ${{ matrix.args }}
- if: runner.os == 'Linux'
- if: ${{ runner.os == 'Linux' && matrix.goversion == '1.21.x' }}
Copy link
Contributor

@ajnavarro ajnavarro Sep 21, 2023

Choose a reason for hiding this comment

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

this might be a problem when we update go versions and 1.21 is not supported anymore, we can lose coverage execution. Maybe add go version to the flags?

Copy link
Member Author

@moul moul Sep 21, 2023

Choose a reason for hiding this comment

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

yeah, I'm the one who made the 3 last go version bumps and I basically made sure to update all the references I can find.

at some point, I was considering adding a variable.yml file with "latest_go" and "previous_go" variable; or maybe we can add a quick misc/check-go-versions.sh script to assist us or to check in the CI.

Signed-off-by: moul <94029+moul@users.noreply.github.com>
@moul
Copy link
Member Author

moul commented Sep 21, 2023

The flakiness issue appears to be resolved, as seen in my fork:

CleanShot 2023-09-21 at 19 11 36

Edit: I might have been a bit hasty in my assessment. I've just initiated new builds. I'll keep you updated.

Edit 2: Upon review, I believe this PR effectively makes Codecov visible while minimizing false positives. I recommend we proceed with this to resolve the current impasse. Subsequently, we can dedicate more time to refining QA and DX.

Signed-off-by: moul <94029+moul@users.noreply.github.com>
@moul moul merged commit cfefb3b into gnolang:master Sep 22, 2023
187 checks passed
@moul moul added this to the 🚀 main.gno.land (required) milestone Sep 22, 2023
gfanton pushed a commit to gfanton/gno that referenced this pull request Nov 9, 2023
I've conducted tests on my fork, which you can find at
https://github.com/moul/gno.

The 'if' condition draws inspiration from @ajnavarro's contributions, so
a shoutout to him for that.

Additionally, @ajnavarro pointed out gaps in our testing for certain
packages. We need to strategize on expanding our test coverage to
address these overlooked areas.

I suggest waiting for his alternative work before considering merging
mine.

---------

Signed-off-by: moul <94029+moul@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 bug Something isn't working
Projects
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants