Page MenuHomePhabricator

Allow PhutilUnitTestEngine to execute tests from a single path
ClosedPublic

Authored by joshuaspence on May 4 2015, 10:38 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 22, 11:37 PM
Unknown Object (File)
Fri, Nov 22, 11:30 PM
Unknown Object (File)
Fri, Nov 22, 10:46 PM
Unknown Object (File)
Tue, Nov 19, 12:40 AM
Unknown Object (File)
Fri, Nov 15, 3:03 PM
Unknown Object (File)
Thu, Nov 14, 9:47 PM
Unknown Object (File)
Tue, Nov 12, 9:57 AM
Unknown Object (File)
Mon, Nov 11, 12:54 PM
Subscribers
Tokens
"Grey Medal" token, awarded by epriestley.

Details

Summary

Fixes T8042. Changes the way that PhutilUnitTestEngine discovers unit tests. In particular, if you only modify a single test case then there is no reason to run all other test cases within the same directory (which is the current behavior).

Test Plan

Added some unit tests.

Diff Detail

Repository
rARC Arcanist
Branch
master
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 6282
Build 6304: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence retitled this revision from to Allow PhutilUnitTestEngine to execute tests from a single path.
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
epriestley edited edge metadata.

I think I caught an issue? At least, I don't understand the logic.

It would maybe be nice to have some kind of flag which shows which tests will be run without executing them, although maybe this is a task for another time (e.g., after T5568).

src/unit/engine/PhutilUnitTestEngine.php
190–191

I would expect && $library_path == $library_root to fail immediately in many cases and throw away most of the test we'd otherwise find.

e.g., if you touch /a/b/c/d/e, we'd previously walk up and include /a/b, but now we'll fail out at /a/b/c/d or /a/b/c/d/e or something?

This revision now requires changes to proceed.May 19 2015, 2:06 PM
epriestley edited edge metadata.

Nice!

src/unit/engine/PhutilUnitTestEngine.php
44–46

I think id() isn't necessary on newv(), only on new X(). PHP grammar lols

This revision is now accepted and ready to land.May 25 2015, 4:09 PM
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence marked an inline comment as done.

Oops

This revision was automatically updated to reflect the committed changes.