Don't run the project tests after all

[cole-miller]

It's a bit embarrassing, but I realized after merging #112 and the follow-up PRs that it doesn't make sense for downstream tests of dqlite and go-dqlite to live in the Jepsen workflow. That's because the Jepsen job has a matrix, which "infects" any calling workflow, so the dqlite and go-dqlite test suite will get run many times. Instead, we should have separate workflows in raft, dqlite, and go-dqlite for running downstream test suites and for Jepsen.

Signed-off-by: Cole Miller cole.miller@canonical.com

[freeekanayaka]

I'm slightly concerned about the overhead of running the whole dqlite, go-dqlite and jepsen suites for every raft PR been submitted. I presume that every single push to such a PR would trigger the downstream tests?

Would something weaker be acceptable? For example, assuming we have stable jepsen tests that are always green and that are run at regular interval (which I believe is the case today, at least for "Dqlite Jepsen tests - expected pass"), then if we receive a notification mail about a failing Jepsen run, we can stop the line and look at what happened. We have a merge rate which is well below a PR per day, so it's not too bad.

Or alternatively run the downstream tests when a PR is merged.

I can see why you want a stronger model, just trying to better understand the cost/benefit tradeoffs here.

[MathieuBordere]

Or maybe some way to optionally trigger them, like once we're quite confident the PR is fine, we can trigger the tests down the line.

[freeekanayaka]

Or maybe some way to optionally trigger them, like once we're quite confident the PR is fine, we can trigger the tests down the line.

This would also have the benefit that we would not trigger them for obviously trivial or safe changes.

[cole-miller]

I agree that it's overkill to do this for every push, and that there are some PRs that obviously don't need it, but I do think it provides value to have such checks run before a PR is merged, instead of picking up problems only after merge (for all the same reasons that we have pre-merge checks at all). In practice we already run downstream tests and Jepsen manually for PRs that call for it, like Mathieu's work at canonical/raft#446, this is just about automating that. I think the ideal would be something like this:

• To start with, opening a PR or pushing to a PR branch only causes the project-local tests to run (as now).
• When you've got those checks green, you can somehow kick off the downstream/Jepsen test workflows (or not, if it's an obviously "safe" change).
• Once those pass (if applicable), the PR can be merged.

[edit: so pretty much what Mathieu suggested above, if I understand correctly?]

But I don't know what the best way is to implement that using GHA -- it might need a bot of some kind.

[freeekanayaka]

I agree that it's overkill to do this for every push, and that there are some PRs that obviously don't need it, but I do think it provides value to have such checks run before a PR is merged, instead of picking up problems only after merge (for all the same reasons that we have pre-merge checks at all). In practice we already run downstream tests and Jepsen manually for PRs that call for it, like Mathieu's work at canonical/raft#446, this is just about automating that. I think the ideal would be something like this:

• To start with, opening a PR or pushing to a PR branch only causes the project-local tests to run (as now).
• When you've got those checks green, you can somehow kick off the downstream/Jepsen test workflows (or not, if it's an obviously "safe" change).
• Once those pass (if applicable), the PR can be merged.

That would be a good setup indeed.

[edit: so pretty much what Mathieu suggested above, if I understand correctly?]

Yes, afaiu.

But I don't know what the best way is to implement that using GHA -- it might need a bot of some kind.

A bot might be the only option indeed, short of some kind of clever trick.

[cole-miller]

It looks like this is possible to do using only "vanilla" GitHub Actions, I should have it implemented shortly.