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

Target iroha2-dev branch for Coveralls #4131

Closed
mversic opened this issue Dec 8, 2023 · 6 comments
Closed

Target iroha2-dev branch for Coveralls #4131

mversic opened this issue Dec 8, 2023 · 6 comments
Assignees
Labels
CI iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@mversic
Copy link
Contributor

mversic commented Dec 8, 2023

our tests coverage should be compared against the iroha2-dev branch, not iroha2-stable.

Concerns

I was told that we target iroha2-dev branch because of computational resource concerns since target branch will have to be recomputed every time in case of iroha2-dev. If possible we should reuse latest build of iroha2-dev branch and share it among all active PRs so as to alleviate this a little bit. How big of a deal is to recompute target branch coverage?

@mversic mversic added CI iroha2-dev The re-implementation of a BFT hyperledger in RUST labels Dec 8, 2023
@BAStos525
Copy link
Contributor

We can't select the target branch manually since coverall detects only the default repo branch that is main. But PRs against other base branches are possible and we see them. The problem that it tries to compare every new PR against the very old merger PR from 30Th June. It's weird because we had a similar issue with Codecov. And when I tied to cancel this old build, it went to the error. By the way, I have changed a couple of setting in Coverall and re-cached it. Let's watch if smth could change or we wiill nee to continue to investigate the problem.

@BAStos525
Copy link
Contributor

BAStos525 commented Dec 19, 2023

We don't receive Coveralls PR comments again.
From docs:

"""
I’M NOT GETTING PR COMMENTS.
This can happen when Coveralls can’t find the base build for your PR build. The base build is the Coveralls build for the commit on which your PR is based and, likely, to which it will be merged. This is usually a push build on your default branch. So if that’s missing—for instance, if you’re not building push builds in CI—then you’ll want to make sure you start building those.
"""

I guess to take it back, we have to return the custom Coveralls action settings that could call a branch misbehavior. @mversic please tell if Coverall PR comments are reqired.

@BAStos525
Copy link
Contributor

I think I catch the problem: Coveralls compare each run with the correct iroha2-dev branch base commit, but for every new commit (run) withing the PR it creates the new job that doesn't know about the previous one PR head branch commit. So, we can't determine the summarized diff between head and base commits and as a result, we don't receive the Coveralls PR comment. May be it caused either by PR from fork or force-pushed commit or that i2-dev branch if not a default branch.

I suggest to try Covecov again. @mversic

@BAStos525
Copy link
Contributor

BAStos525 commented Jan 23, 2024

If we take any PR, i.e. this one (#4053)
Then go to check and find coverage/coveralls (pull_request) and click on details that should open Coveralls page. We see COMMIT MESSAGE Merge ef05a9c into 85d03b7. Actual head and base commit SHA.
The problem here that it doesn't give the proper PR comments. I guess due to default branch thing or smth else:

This can happen when Coveralls can’t find the base build for your PR build. The base build is the Coveralls build for the commit on which your PR is based and, likely, to which it will be merged. This is usually a push build on your default branch. So if that’s missing—for instance, if you’re not building push builds in CI—then you’ll want to make sure you start building those.

When we make a PR to dev branch, it impossible to compare PR with stable branch because we put the base branch commit value for compare-ref var manually. But a problem with Coveralls that we don't receive PR comment that is not convenient.

For Codecov it's the issue that it calculates the base as a4d5c9f

It's very old commit and I warn that we can't reset it without Codecov support. I tried to reset but it's just gave me the error that is was unable to reset. Related issue.

@BAStos525
Copy link
Contributor

I am pleasantly surprised that coveralls has started to send PR comments with coverage info. Example.
Also it seems that PR head commit is compared with the latest base iroha2-dev branch commit.
Can we resolve this issue? @mversic

@BAStos525
Copy link
Contributor

I think it works now for our luck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

2 participants