Details
See green lights over your new unit tests:
arc unit src/applications/calendar/import/__tests__/CalendarImportTestCase.php
Diff Detail
- Repository
- rP Phorge
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Hard to tell if it checks all and anything that could ever be tested (ICS files can be a looot of phun), on the other hand I cannot come up with any downsides of merging this either as it could always be expanded or adjusted and it passes currently for me, so I'll accept.
Only thing I was wondering if this should be located one level higher at src/applications/calendar/__tests__/ but...I guess not.
[acko@foo phorge (arcpatch-D25767 $)]$ ../arcanist/bin/arc unit src/applications/calendar/import/__tests__/CalendarImportTestCase.php
PASS 1.4s CalendarImportTestCase::testIcsFileImportWithGuestThatIsHost
COVERAGE REPORT
94% src/applications/calendar/import/__tests__/CalendarImportTestCase.phpGood question. The documentation is not "super-opinionated" about this - https://we.phorge.it/book/contrib/article/unit_tests/
At the moment this is a specific unit test against the business logic in the import directory, so, triggering for changes only on these relevant files:
import/PhabricatorCalendarICSFileImportEngine.php import/PhabricatorCalendarICSURIImportEngine.php import/PhabricatorCalendarICSImportEngine.php import/PhabricatorCalendarImportEngine.php
And premising that the application has this structure with a couple of specific unit tests for the parser ICS + separated ones for the parser data, so it seems good to create "specific" unit test directories:
$ cd src/applications/calendar $ find -name __tests__ ./__tests__ ./parser/ics/__tests__ ./parser/data/__tests__
In short, this specific unit test against imports can stay in the import directory, to be triggered only if strictly-connected business-logic changes, also I think to save some development time if the user touches irrelevant things.
Eventually we can move it up in the future, to run it more often.