protobuf icon indicating copy to clipboard operation
protobuf copied to clipboard

Update GHA tests to return single pass/fail signal at the end

Open deannagarcia opened this issue 1 year ago • 1 comments

Before this PR, we stored a list internally of tests that must pass on presubmit and tried to keep it up to date.

This PR moves that information keeping into GitHub by adding a 'run-on-presubmit' variable to most testing matrices to allow authors to specify which of their tests should serve as submission blockers. During presubmit, tests that were specified to not run on presubmit will not be run and their names will be prefixed with "[SKIPPED]". All continuous only tests will be suffixed with "(Continuous)".

At the end of running all the tests, we have a single "All Blocking Tests" signal that will tell us whether all of the necessary tests have passed (either for presubmit or continuous based on how the test was triggered).

I've tested this from a different branch here and from a different fork here. These should be the same and are as far as I can tell.

I also have a continuous test run here which runs the entire test suite.

deannagarcia avatar Jun 20 '24 21:06 deannagarcia

Note: we should make sure the dashboard and copybara are ready for this before we merge. You'll probably need to update the copybara configs atomically too

mkruskal-google avatar Jun 20 '24 22:06 mkruskal-google

Note: we should make sure the dashboard and copybara are ready for this before we merge. You'll probably need to update the copybara configs atomically too

Agreed and was planning on discussing more with you after approval to decide the best way to get this in. For copybara, I'm thinking it might be best to submit this as a CL so we can include the copybara config update with it. Does that work?

And does anything specific need to be done for the dashboard?

deannagarcia avatar Jul 01 '24 21:07 deannagarcia

Note: we should make sure the dashboard and copybara are ready for this before we merge. You'll probably need to update the copybara configs atomically too

Agreed and was planning on discussing more with you after approval to decide the best way to get this in. For copybara, I'm thinking it might be best to submit this as a CL so we can include the copybara config update with it. Does that work?

And does anything specific need to be done for the dashboard?

Unfortunately copybara uses the config from HEAD for blocking tests, so you'll still have to force it in and won't be able to validate it until after submission. However we should still probably do that atomically to avoid breaking people in the interim.

For the dashboard we probably have to tweak the GCP setup to filter using the new criteria instead of the old hardcoded list. We might be able to get away with not updating the dashboard itself at all though

mkruskal-google avatar Jul 12 '24 01:07 mkruskal-google