Proposal: Propagate test failure status to shell in predictive

Asherah Connorrequested to merge
push-kwzxuzqxxsmw into main

What does this merge request do and why?

Hi! Something that's tripped me up a couple times is that gdk predictive doesn't return non-zero when there are test failures. This can get me particularly because of how it runs spec groups by helper: sometimes the last output in my terminal looks like this:

warning: parser/current is loading parser/ruby33, which recognizes 3.3.10-compliant syntax, but you are running 3.3.9.
Please see https://github.com/whitequark/parser#compatibility-with-ruby-mri.
Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}
.........

Finished in 0.01126 seconds (files took 1.72 seconds to load)
9 examples, 0 failures

Randomized with seed 59131

ℹ️  No changes were detected in JavaScript files. Nothing to do.

This plus a zero exit code makes me think the tests passed, whereas in fact if I scroll up, I see there were errors, in a different grouped-by-spec-helper group!

This MR is a proposal to have gdk predictive return non-zero if any of the tests run fail.

We aggregate the success? status of each spec group and return it as the overall result of GDK::Predictive::Rspec#execute. We also return the success status from GDK::Predictive::Jest#execute. Finally, GDK::Command::Predictive#run mixes in the return results of each of those to the success local variable.

We still always run all the groups and both sets of predictive tests, even if there's a failure midway — we just make sure the overall return result reflects if there was any failure. This makes gdk predictive exit with a non-zero status in such situations.

I haven't checked for any relevant documentation changes, since I wanted feedback on the idea first before I got it to review-ready. Assuming it looks OK, the checklist will get run through before then!

How to set up and validate locally

  1. Check out the branch.
  2. Run gdk predictive --yes. Hopefully everything passes (or nothing runs!) and you get a zero (success) exit result.
  3. Modify some code or test such that predictive will run the test, and the test will fail.
  4. Run gdk predictive --yes. You should get a non-zero (failure) exit result to the shell.

Impacted categories

The following categories relate to this merge request:

Merge request checklist

  • This MR references an issue describing the change.
  • This change is backward compatible. If not, include steps to communicate to users.
  • Tests added for new functionality. If not, raise an issue to follow-up.
  • Observability added/updated (logging, metrics, tracing).
  • Documentation added/updated.
  • Announcement added for notable changes.
  • gdk doctor test added.
Edited by Asherah Connor

Merge request reports