ci: run Codecov for commits to main #1751
Conversation
Codecov Report
Additional details and impacted files
|
Now we test each commit to main, this isn't required
There was a problem hiding this comment.
The motivation for the scheduled test of main and its low frequency was to keep the "tests" badge green, actually. We don't allow tests to fail on main (though that's only partially tested, due to the concurrency issues in always testing exactly what will be in the main branch after merging), so the badge would, in principle, always be green, anyway, but it was grey because of the lack of explicit tests. The 30 day frequency was to avoid spending too much of our resources on the aesthetics of a green badge.
To keep that badge non-gray, we'll need some CI tests to run on main occasionally. Doing it with every commit nearly duplicates the PR branch testing. Maybe it could be done with a smaller matrix? Maybe just one platform-Python version?
But doing both the tests and the Codecov on main is desirable.
|
Good, that settles it. It might actually be possible to check whether the merged PR used the same base commit as main. If we can do that, then we could, in those cases, assume that the coverage result would match the merged repo. Otherwise, we can run a simple single-matrix-element build (Linux py37) to compute the coverage for the merged commit. If this is not possible, then we can just keep periodic full testing on main, and add a new per-commit single-matrix-element build as suggested in the previous paragraph. The disadvantage of the single element case is that we might miss changes that are platform/Python version specific. We could choose random configurations such that we cover more matrix elements over the course of the month... but I see that being problematic for determinism reasons. At the end of the day, do we make per-platform changes more frequently than monthly? I suspect not. |
agoose77
changed the title
|
I've settled on just running a single Codecov job for commits to main. Every month we'll run a full build, which will update our coverage baseline in cases where we made coverage-altering per-platform changes. I think these are probably rare enough tha t we don't need to go to the extra effort of testing a larger matrix, or determining whether we can reuse coverage results. |
There was a problem hiding this comment.
This new test does not run monthly, but adding 1 extra task upon PR-merger doesn't significantly increase resource usage, since we're running ~10 for every commit already. It looks like this will provide a Codecov baseline and a quick test of main itself (as opposed to the last commit in the branch).
That seems like a good compromise between leaving actual-main untested (i.e. assuming that all PRs merge into something correct) and fully testing main on every PR merger, or requiring every PR to be up-to-date with main before merging and requiring all of their tests to pass (which essentially serializes testing and merging).
Let's go with this!
This PR enables
build-test.ymlandtests.ymlfor each commit to main. This addresses two problems:This is the "obvious" way to tackle both of these problems, but it is not necessarily the nicest for the environment.
So, there are other ways of tackling this:
codecovworkflow for each commit to main. Only runbuild-test.ymlandtests.ymlat a reduced frequency, e.g once every couple of days.codecovworkflow for each commit to main. Also, runtests.ymlfor each commit to main.@jpivarski do you have a preference here?
Fixes #1745