Further improvements to test discovery
ClosedPublic
Actions

Authored by joshuaspence on Jun 7 2015, 8:58 AM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Commits

rARC3c3438714b43: Further improvements to test discovery

Summary

I found another issue with T8042... it seems that tests from the root directory (i.e. src/__tests__ are not being discovered properly). The handling of this case ($library_path being the library root directory) can probably be improved, and I am open to suggestions. Depends on D13202.

Test Plan

Added to existing test cases.

Diff Detail

Repository

rARC Arcanist

Branch

master

Lint

Lint Warnings

Severity	Location	Code	Message
Warning	src/unit/engine/PhutilUnitTestEngine.php:211	TXT3	Line Too Long

Unit

Test Failures

Build Status

Buildable 6734
Build 6756: [Placeholder Plan] Wait for 30 Seconds

Unit TestsFailed
View All

	Time	Test
	5 ms	PhpunitTestEngineTestCase::testSearchLocations
	0 ms	ArcanistLibraryTestCase::testEverythingImplemented
	28 ms	ArcanistLibraryTestCase::testLibraryMap
	50 ms	ArcanistLibraryTestCase::testMethodVisibility
	0 ms	PhutilUnitTestEngineTestCase::testFailSkip
		View Full Test Results (1 Failed · 8 Passed)

Event Timeline

joshuaspence updated this revision to Diff 31869.Jun 7 2015, 8:58 AM

joshuaspence retitled this revision from to Further improvements to test discovery.

joshuaspence updated this object.

joshuaspence edited the test plan for this revision. (Show Details)

joshuaspence added a reviewer: epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptJun 7 2015, 8:58 AM

Herald added subscribers: epriestley, Korvin. · View Herald Transcript

joshuaspence mentioned this in D13186: Fix some method visibilities.Jun 7 2015, 9:06 AM

Since the complexity mostly arises out of dirname()'s behavior being a bit of a mess, maybe this would be cleaner:

Update PhabricatorFilesystem::walkToRoot() to take a second parameter (like $root) which specifies a parent directory where iteration should stop.
Get some test coverage on that, since it'll be easy to test properly outside of need to do a bunch of unit test stuff (e.g., path same as root, path outside root, path a symlink into root).
Replace the manual directory fiddling here with walkToRoot().

I think that would make this a bit simpler and less error prone.

src/unit/engine/PhutilUnitTestEngine.php
201	Do we now sometimes end up with the key `libname:.` and sometimes with the key `libname:` for root tests?
211	The unlikely path `0` will do the wrong thing here (output will be `__tests__/`, should be `0/__tests__/`).