Page MenuHomePhabricator

Allow unit test engines to render their own results
AbandonedPublic

Authored by joshuaspence on Sep 2 2015, 8:30 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 24, 5:08 PM
Unknown Object (File)
Wed, Nov 20, 5:31 PM
Unknown Object (File)
Sat, Nov 16, 6:49 AM
Unknown Object (File)
Sat, Nov 16, 5:21 AM
Unknown Object (File)
Tue, Nov 12, 10:07 AM
Unknown Object (File)
Sat, Nov 9, 11:45 AM
Unknown Object (File)
Sat, Nov 9, 10:02 AM
Unknown Object (File)
Sat, Nov 9, 10:02 AM
Subscribers

Details

Summary

Fixes T9131. As of D13579, unit tests results were no longer rendered progressively. The issue here is that the rendering is being done in ArcanistConfigurationDrivenUnitTestEngine rather than in the child test engines, and only after $engine->run() had returned (i.e. after all tests within a test engine had completed).

Test Plan

Ran arc unit, saw progressive results.

Diff Detail

Repository
rARC Arcanist
Branch
master
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 8853
Build 10346: Run Core Tests
Build 10345: arc lint + arc unit

Event Timeline

joshuaspence retitled this revision from to Allow unit test engines to render their own results.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.

@epriestley, it looks like this was landed (and squashed) when I used "land revision" for D14487. Is that expected? Locally, I had submitted D14487 on top of D14037, although there's no logical relatiionship between the two diffs.

As mentioned, this was inadvertently landed in rARC66ab1c955d27ff7875f62eb51a0bd6c4770a7ae3. @epriestley, we can revert this commit of you want and I will resubmit the diffs.

Yes, that's the expected behavior (just like arc land would try to do if you had the two changes on top of one another).

We can just leave this in, I guess.

Yes, that's the expected behavior (just like arc land would try to do if you had the two changes on top of one another).

I think that this is not desirable behavior. In this particular case, I completely forgot that I had submitted the three diffs on top of each other, and so clicking "Land Revision" and seeing all three squashed into one was surprising. If I had used arc land locally then I could have verified the state before pushing.

The button will cease to work in this case in the future, rejecting the land because it would land unlanded, unreviewed commits. But will always mean "land this state", just like arc land does.

The button will cease to work in this case in the future, rejecting the land because it would land unlanded, unreviewed commits. But will always mean "land this state", just like arc land does.

Ah that's right. OK, that seems reasonable.