Page MenuHomePhorge

Added infrastructure tests for Activesync/CalDav/WOPI/Roundcube & fixed the well-known redirect for caldav.
ClosedPublic

Authored by mollekopf on Jan 16 2023, 2:34 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Oct 2, 11:18 AM
Unknown Object (File)
Wed, Oct 2, 2:02 AM
Unknown Object (File)
Fri, Sep 27, 9:15 AM
Unknown Object (File)
Tue, Sep 24, 1:22 AM
Unknown Object (File)
Fri, Sep 13, 8:41 AM
Unknown Object (File)
Fri, Sep 13, 8:39 AM
Unknown Object (File)
Fri, Sep 13, 8:39 AM
Unknown Object (File)
Thu, Sep 12, 11:12 AM
Subscribers

Diff Detail

Repository
rK kolab
Lint
Lint Errors
SeverityLocationCodeMessage
Errorsrc/app/Backends/DAV.php:187PHPCS.E.PSR12.ControlStructures.ControlStructureSpacing.FirstExpressionLinePSR12.ControlStructures.ControlStructureSpacing.FirstExpressionLine
Errorsrc/app/Backends/DAV.php:306PHPCS.E.PSR12.Operators.OperatorSpacing.NoSpaceAfterPSR12.Operators.OperatorSpacing.NoSpaceAfter
Errorsrc/app/Backends/DAV.php:362PHPCS.E.PSR12.Operators.OperatorSpacing.NoSpaceAfterPSR12.Operators.OperatorSpacing.NoSpaceAfter
Errorsrc/tests/Infrastructure/ActivesyncTest.php:17phpstanInstantiated class Syncroton_Wbxml_Encoder not found.
Errorsrc/tests/Infrastructure/ActivesyncTest.php:20phpstanCall to method encode() on an unknown class Syncroton_Wbxml_Encoder.
Errorsrc/tests/Infrastructure/ActivesyncTest.php:30phpstanInstantiated class Syncroton_Wbxml_Decoder not found.
Errorsrc/tests/Infrastructure/ActivesyncTest.php:31phpstanCall to method decode() on an unknown class Syncroton_Wbxml_Decoder.
Errorsrc/tests/Infrastructure/DavTest.php:78phpstanUnreachable statement - code above always terminates.
Unit
No Test Coverage
Build Status
Buildable 41363
Build 16586: arc lint + arc unit

Event Timeline

mollekopf created this revision.

Include the roundcube test and fixed some lint issues

mollekopf added a reviewer: Restricted Project.Jan 23 2023, 9:32 PM
machniak subscribed.
machniak added inline comments.
src/tests/Infrastructure/ActivesyncTest.php
41

getTestUser() is using fake queue. So, you'd have to execute the job(s) manually. Or using IMAP::createUser($user);

47

This should be configurable, please.

72

Use assertStringContainsString().

This revision now requires changes to proceed.Jan 24 2023, 10:15 AM
mollekopf retitled this revision from Added infrastructure tests for our Activesync/CalDav/wopi infrastructure to Added infrastructure tests for Activesync/CalDav/WOPI/Roundcube & fixed the well-known redirect for caldav..
mollekopf edited the summary of this revision. (Show Details)

Test cleanup to work with test users.

Now includes an nginx fix because the commits were too cumbersome to untangle.

machniak added inline comments.
src/tests/Infrastructure/ActivesyncTest.php
48

I suppose this common code could be moved to getTestUser() (optional). Also, we should not use LDAP::createUser() if LDAP is disabled.

71

Should we use setUpBeforeClass() if we use tearDownAfterClass()? Also, we definitely want to remove the created user.

112

Debug code.

src/tests/Infrastructure/AutodiscoverTest.php
19

This too needs to be configurable. The chwala and webmail locations too. Then maybe instead of creating a separate config file for each "self" service we should put them somewhere together, e.g. in services.php or another file?

src/tests/Infrastructure/DavTest.php
53

Just use $this->deleteTestUser($this->user->email).

This revision now requires changes to proceed.Jan 25 2023, 10:22 AM
mollekopf added inline comments.
src/tests/Infrastructure/ActivesyncTest.php
71

They are static so I can't set member variables. That's why I ended up using setUp.

src/tests/Infrastructure/AutodiscoverTest.php
19

I understand the sentiment, but until we actually use the configs elsewhere I don't think it matters (which is why I didn't do it in the first place).

I like the idea of having a services.php file, and another variable in there also doesn't hurt, so I'll do that.

src/tests/Infrastructure/DavTest.php
53

tearDownAfterClass is a static function.

We can make deleteTest* and getTest* methods static.

Made uri's configurable, some cleanup, centralized test account setup and cleanup.

mollekopf added inline comments.
src/tests/TestCaseTrait.php
576

I thought this works, but it doesn't.

phpunit seems to sometimes reinstantiate the entire class between runs (member variables are lost), and beforeApplicationDestroyed is called on every tearDown().

So, phpunit makes it impossible to implement a setup first, run all tests, teardown last appraoch.

I'm going to stuff all tests into a single test function and be done with it.

This is what I've arrived at.

Unfortunately it's just not really possible I think to implement the user setup/teardown in a sensible fashion in phpunit without more intrusive changes.
The crux is that the laravel integration uses setUp/tearDown to setup and teardown the laravel framework, so outside of it we just can't really do anything (no db interaction anyways).
So we're stuck with lazy initialization, plus using a test and relying on the default ordering for teardown.
This now at least works.

In the process I also found an issue with the activesync test that would always return no folders, which I tracked to the initialization not being run after the account removal + creation,
because syncroton already knew the deviceid, it assumed the initial setup was done, but the metadata was empty of course (resulting in all folders being filtered including inbox).
Fixed by using a random deviceid.

I'd probably change a few things, but it's nothing that can't be done later.

This revision is now accepted and ready to land.Jan 30 2023, 3:56 PM