Closed G-Rath closed 11 months ago
Ok so it looks like the JRuby and "oldest supported RuboCop version" jobs are already failing which this change has revealed - I've added an env variable to control if test failures should actually be surfaced to use in those two jobs so that this PR can be landed as a dedicated change, and then those jobs can be addressed in their own dedicated PRs (since "mostly working CI" is better than "not working CI" and I have no context on either of the failures so expect they'll take more time to resolve).
I've kept this as a separate commit in case you want to do something different @koic, but if you're happy with this path I can squash it.
Also it looks like MultipleAssertionsTest#test_generates_a_todo_based_on_the_worst_violation
is flakey - I've confirmed this locally: running multiple times, every so often I get a failure with an incorrect todo count; again something to resolve in a dedicated PR - for now just rerunning that job for this PR should get it passing.
I realised while working on my recent PRs that CI wasn't actually failing when
bundle exec rake [test]
reported errors - turns out that tests are being invoked bysystem
which by default does not do any error handling; instead it sets$?
to the exit code which can then be checked.Ruby 2.6 added an
exception
option which whentrue
causessystem
to raise an exception if the command exits with a non-zero code, making this very easy to address.I've checked the codebase and confirmed these are the only uses of
system
.Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.md
if the new code introduces user-observable changes. See changelog entry format for details.~