Page MenuHomePhorge

Calendar Import: add unit tests to cover participants
ClosedPublic

Authored by valerio.bozzolan on Aug 8 2024, 10:26.

Details

Summary

Add unit tests to easily double-check matched participants in imported calendar events.

This will simplify the addition of future features without the risk to break older workflows.

Ref T15564

Closes T15905
Closes T15906

Test Plan

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

Either INVITEED_* is a typo or I don't get what the second E stands for. :)

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.php
This revision is now accepted and ready to land.Aug 18 2024, 18:41

Good 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.