Improve the `PhutilInfrastructureTestCase` unit tests
ClosedPublic
Actions

Authored by joshuaspence on Jul 5 2014, 6:54 AM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Commits

rPHU4620dbb05d75: Improve the `PhutilInfrastructureTestCase` unit tests

Summary

Improve the PhutilInfrastructureTestCase unit tests such that they will fail if any of the following conditions are satisfied:

A symbol referenced in the __phutil_library_map__.php file no longer exists.
A symbol exists in the library but is not referenced within the __phutil_library_map__.php file.
A symbol extends from a different parent symbol than that declared in the __phutil_library_map.php file.

Test Plan

Added a new function. Ran arc unit and observed the test failure.

Diff Detail

Repository

rPHU libphutil

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

joshuaspence updated this revision to Diff 23574.Jul 5 2014, 6:54 AM

joshuaspence retitled this revision from to Improve the `PhutilInfrastructureTestCase::testEverythingImplemented` unit test.

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 TranscriptJul 5 2014, 6:54 AM

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

Harbormaster completed remote builds in B1523: Diff 23574.Jul 5 2014, 6:55 AM

joshuaspence added a parent revision: D9822: Move `PhutilLibraryMapBuilder` into the standard directory structure.Jul 5 2014, 6:55 AM

Rename test method

Harbormaster completed remote builds in B1524: Diff 23575.Jul 5 2014, 6:58 AM

joshuaspence added inline comments.Jul 5 2014, 6:58 AM

src/moduleutils/PhutilLibraryMapBuilder.php
70–82	I removed this because it simplifies the implementation. Plus, I doubt this is commonly used.

I think we should retain the phase where the code is loaded. Particularly, it detects things like:

syntax error in some file;
subclass extending an abstract base class but not implementing all of the required methods.

It seems like we can no longer detect those issues?

This revision now requires changes to proceed.Jul 5 2014, 2:48 PM

Rename test method
Retain the original test case as well.
Improve assertion message

Harbormaster completed remote builds in B1526: Diff 23582.Jul 5 2014, 3:17 PM

epriestley accepted this revision.Jul 5 2014, 3:20 PM

epriestley edited edge metadata.

This revision is now accepted and ready to land.Jul 5 2014, 3:20 PM

joshuaspence retitled this revision from Improve the `PhutilInfrastructureTestCase::testEverythingImplemented` unit test to Improve the `PhutilInfrastructureTestCase` unit tests.Jul 5 2014, 3:20 PM

joshuaspence updated this object.

joshuaspence edited edge metadata.