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

Measure test coverage with GitHub Actions #755

Merged
merged 3 commits into from
Nov 27, 2020
Merged

Conversation

ilammy
Copy link
Collaborator

@ilammy ilammy commented Nov 27, 2020

Migrate the coverage step from CircleCI to GitHub Actions. It's pretty easy: compile Themis with gcov instrumentation, run the tests, use lcov to extract coverage data, post results using official Action.

Compared to CircleCI, the difference is in how we exclude the tests directory. For some reason the version of lcov available on GitHub's runners handles relative paths slightly differently.

(Also, if I understand the docs correctly, Coveralls can (will?) post coverage results in pull requests, thanks to that action.)

On CircleCI retirement

Now that GitHub Actions have stolen the last job CircleCI had, it's no longer necessary on the master branch. However, CircleCI is still running nightly builds on the stable branch so we can't turn it off just like that.

Due to the way CircleCI works, it needs to see the .circleci/config.yml file on the default branch of the repo (i.e., master). However, we don't need it to do anything. So put there the minimal valid config and let CircleCI rest. It had worked hard for us all these days, after all.

Once Themis 0.14 is out, GitHub Actions configuration will be applied to the stable branch, and only then CircleCI can be deconfigured and finally retired.

On GoThemis coverage

Also note that these is no replacement for goveralls execution that is removed in this commit. It does not seem to be working right now, and no one knows when exactly it stopped working. Well, we can't replace it easily right now so GoThemis is in the same boat with other wrappers: that is, test coverage is collected only for Themis Core. Maybe future work on Themis 0.14 will add more coverage coverage before the release.

Checklist

  • Change is covered by automated tests
  • The coding guidelines are followed
  • Public API has proper documentation
  • Changelog is updated
  • x86_64 job is marked optional in master branch settings

Migrate the coverage step from CircleCI to GitHub Actions. It's pretty
easy: compile Themis with gcov instrumentation, run the tests, use lcov
to extract coverage data, post results using official Action.

Compared to CircleCI, the difference is in how we exclude the "tests"
directory. For some reason the version of lcov available on GitHub's
runners handles relative paths slightly differently.
Now that GitHub Actions have stolen the last job CircleCI had, it's no
longer necessary on the "master" branch. However, CircleCI is still
running nightly builds on the "stable" branch so we can't turn it off
just like that.

Due to the way CircleCI works, it needs to see the ".circleci/config.yml"
file on the default branch of the repo (i.e., "master"). However, we
don't need it to do anything. So put there the minimal valid config and
let CircleCI rest. It had worked hard for us all these days, after all.

Once Themis 0.14 is out, GitHub Actions configuration will be applied
to the "stable" branch, and only then CircleCI can be deconfigured and
finally retired.

(Also note that these is no replacement for "goveralls" execution that
is removed in this commit. It does not seem to be working right now, and
no one knows when exactly it stopped working. Well, we can't replace it
easily right now so GoThemis is in the same boat with other wrappers:
that is, test coverage is collected only for Themis Core. Maybe future
work on Themis 0.14 will add more coverage coverage before the release.)
@ilammy ilammy added infrastructure Automated building and packaging tests Themis test suite labels Nov 27, 2020
@ilammy
Copy link
Collaborator Author

ilammy commented Nov 27, 2020

(Also, if I understand the docs correctly, Coveralls can (will?) post coverage results in pull requests, thanks to that action.)

Yes, it can, but it needs to be enabled explicitly. I'm not a great fan of bots spamming comments into PRs, so I'd leave that disabled.

However, I have enabled status API usage there. Starting with the next build, Coveralls should be phoning GitHub with coverage analysis results, failing the build on any of the following events:

  • coverage falls below 70% (currently it's 84%)
  • coverage drops by more than 10% with a single change

@iamnotacake
Copy link
Contributor

coverage drops by more than 10% with a single change

What exactly is a single change? Single commit in master after squashing+merging everything from some PR?

And the coverage: XX% badge on top of README.md will now use data from GitHub Actions?

@ilammy
Copy link
Collaborator Author

ilammy commented Nov 27, 2020

@iamnotacake,

What exactly is a single change?

I'm not sure, let's find out! 🤠

On a serious note, Coveralls docs don't seem to go into much detail here. I'd assume this is measured as a difference between the baseline for the branch and the next submission for. So its granularity is a push. Submit a PR with 10% loss over the target branch – denied. Submit an update within a PR which crosses the limit – denied. Merge it regardless – still red.

And the coverage: XX% badge on top of README.md will now use data from GitHub Actions?

Right. It sources the data from Coveralls, so it's not going to change. It does not care which CI submits results to Coveralls.

Copy link
Contributor

@vixentael vixentael left a comment

Choose a reason for hiding this comment

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

I marked x86 job optional

@ilammy
Copy link
Collaborator Author

ilammy commented Nov 27, 2020

Yup, here are the status checks:

Screenshot 2020-11-28 at 07 05 31

@ilammy ilammy merged commit c2bef98 into master Nov 27, 2020
@ilammy ilammy deleted the ilammy/remove-circleci branch November 27, 2020 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Automated building and packaging tests Themis test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants