"Arc diff" does not stop/ask on unit test failures

Bug
#1

Using arc diff, it runs the tests, but does not stop or ask me to provide an explanation if the unit tests fail.

With an older version, the message was:

UNIT ERRORS  Unit testing raised errors!
Unit test results include failures! Provide explanation to continue or press
Enter to abort.
    Explanation: 
...

With the most recent, it just continues:

UNIT ERRORS  Unit testing raised errors!
 SKIP STAGING  No staging area is configured for this repository.
Updated an existing Differential revision:
...

Maybe this is related? 'arc diff' doesn't stop at lint failure

Is there a (new) setting I need to enable to have the old behavior?

Reproduction Instructions

  • add unit testing for a repo
  • create a failing unit test
  • do arc diff with including this test

Phabricator/Arcanist Version
arcanist b2e715fc5a9ccd36048ae469850f293b0780403a (13 Jan 2021)

1 Like
#2

It seems this was introduced with this commit:

The description reads as if this was intended?

Ref T13544. Long ag> o, “arc diff” started prompting the user to provide “excuses” when they submitted changes with failing lint or unit tests.

At the time, “arc” was generally more heavy-handed and the review workflow had fewer points where it provided feedback about lint and test issues.

As the workflow has evolved, there is now significantly more feedback (promotion behavior from Draft in Differential, warnings on “arc land”, etc). These days, these prompts feel archaic and like they’re just getting in the way.

When lint/unit have Harbormaster-triggered components, this prompt is also too early (since Harbormaster tests may fail or raise lint messages later). A modern version of this would look more like putting revisions in some kind of locked state until authors explain issues. It’s possible that’s worth building, but I’d like to see more interest in it. I suspect this feature is largely just a “nag” feature these days with few benefits.

Is there some way that I can have arc ask me if I want to continue with a failing unit-test? Or do you have a suggestion how to handle failing unit-tests nicely?

#3

You could maybe run arc unit && arc diff to have it stop you?

#4

This is not intended and the behavior will be reverted to the previous behavior in a future version of arc.

#5

Good to know, thanks!

Meanwhile I’ll manage by doing the arc unit before, but that results in the unit tests being executed twice, which might take a bit more time.

#6

Is there a task to follow up on fixing this regression? I had a brief look at Arcanist · Workboard and did not find anything.

#7

There’s no specific task for this, but it’s under the general umbrella of https://secure.phabricator.com/T13544.