Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some phpunit test warnings #34819

Merged
merged 7 commits into from
Dec 6, 2022
Merged

Fix some phpunit test warnings #34819

merged 7 commits into from
Dec 6, 2022

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Oct 26, 2022

The NODB tests currently have plenty of warnings that do not fail the tests but are cases that are not running properly:

  • Test\Authentication\Token\PublicKeyTokenProviderTest
  • OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest
  • OCA\DAV\Tests\unit\BackgroundJob\CleanupInvitationTokenJobTest
  • Test\Encryption\UtilTest
  • OCA\DAV\Tests\unit\CalDAV\CalendarImplTest

Once fixed we should probably add the failOnWarning="true" option to our phpunit config.

There were 50 warnings:

1) Test\Authentication\Token\PublicKeyTokenProviderTest::testGenerateToken
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

2) Test\Authentication\Token\PublicKeyTokenProviderTest::testGenerateTokenNoPassword
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

3) Test\Authentication\Token\PublicKeyTokenProviderTest::testGenerateTokenLongPassword
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

4) Test\Authentication\Token\PublicKeyTokenProviderTest::testGenerateTokenInvalidName
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

5) Test\Authentication\Token\PublicKeyTokenProviderTest::testUpdateToken
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

6) Test\Authentication\Token\PublicKeyTokenProviderTest::testUpdateTokenDebounce
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

7) Test\Authentication\Token\PublicKeyTokenProviderTest::testGetTokenByUser
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

8) Test\Authentication\Token\PublicKeyTokenProviderTest::testGetPassword
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

9) Test\Authentication\Token\PublicKeyTokenProviderTest::testGetPasswordPasswordLessToken
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

10) Test\Authentication\Token\PublicKeyTokenProviderTest::testGetPasswordInvalidToken
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

11) Test\Authentication\Token\PublicKeyTokenProviderTest::testSetPassword
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

12) Test\Authentication\Token\PublicKeyTokenProviderTest::testSetPasswordInvalidToken
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

13) Test\Authentication\Token\PublicKeyTokenProviderTest::testInvalidateToken
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

14) Test\Authentication\Token\PublicKeyTokenProviderTest::testInvaildateTokenById
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

15) Test\Authentication\Token\PublicKeyTokenProviderTest::testInvalidateOldTokens
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

16) Test\Authentication\Token\PublicKeyTokenProviderTest::testRenewSessionTokenWithoutPassword
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

17) Test\Authentication\Token\PublicKeyTokenProviderTest::testRenewSessionTokenWithPassword
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

18) Test\Authentication\Token\PublicKeyTokenProviderTest::testGetToken
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

19) Test\Authentication\Token\PublicKeyTokenProviderTest::testGetInvalidToken
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

20) Test\Authentication\Token\PublicKeyTokenProviderTest::testGetExpiredToken
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

21) Test\Authentication\Token\PublicKeyTokenProviderTest::testGetTokenById
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

22) Test\Authentication\Token\PublicKeyTokenProviderTest::testGetInvalidTokenById
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

23) Test\Authentication\Token\PublicKeyTokenProviderTest::testGetExpiredTokenById
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

24) Test\Authentication\Token\PublicKeyTokenProviderTest::testRotate
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

25) Test\Authentication\Token\PublicKeyTokenProviderTest::testRotateNoPassword
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

26) Test\Authentication\Token\PublicKeyTokenProviderTest::testMarkPasswordInvalidInvalidToken
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

27) Test\Authentication\Token\PublicKeyTokenProviderTest::testMarkPasswordInvalid
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

28) Test\Authentication\Token\PublicKeyTokenProviderTest::testUpdatePasswords
Cannot stub or mock class or interface "Test\Authentication\Token\IDBConnection" which does not exist

29) Test\Encryption\UtilTest::testIsSystemWideMountPoint with data set #0 (false, 'non-matching mount point name', array(), array(), '/mp_another')
Cannot stub or mock class or interface "OCA\Files_External\Lib\StorageConfig" which does not exist

30) Test\Encryption\UtilTest::testIsSystemWideMountPoint with data set #1 (true, 'applicable to all', array(), array())
Cannot stub or mock class or interface "OCA\Files_External\Lib\StorageConfig" which does not exist

31) Test\Encryption\UtilTest::testIsSystemWideMountPoint with data set #2 (true, 'applicable to user directly', array('user1'), array())
Cannot stub or mock class or interface "OCA\Files_External\Lib\StorageConfig" which does not exist

32) Test\Encryption\UtilTest::testIsSystemWideMountPoint with data set #3 (true, 'applicable to group directly', array(), array('group1'))
Cannot stub or mock class or interface "OCA\Files_External\Lib\StorageConfig" which does not exist

33) Test\Encryption\UtilTest::testIsSystemWideMountPoint with data set #4 (false, 'non-applicable to current user', array('user2'), array())
Cannot stub or mock class or interface "OCA\Files_External\Lib\StorageConfig" which does not exist

34) Test\Encryption\UtilTest::testIsSystemWideMountPoint with data set #5 (false, 'non-applicable to current user's group', array(), array('group2'))
Cannot stub or mock class or interface "OCA\Files_External\Lib\StorageConfig" which does not exist

35) Test\Encryption\UtilTest::testIsSystemWideMountPoint with data set #6 (true, 'mount point without leading slash', array(), array(), 'mp')
Cannot stub or mock class or interface "OCA\Files_External\Lib\StorageConfig" which does not exist

36) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testAccept with data set "local attendee" (false)
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

37) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testAccept with data set "external attendee" (true)
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

38) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testAcceptSequence with data set "local attendee" (false)
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

39) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testAcceptSequence with data set "external attendee" (true)
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

40) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testAcceptRecurrenceId with data set "local attendee" (false)
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

41) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testAcceptRecurrenceId with data set "external attendee" (true)
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

42) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testAcceptTokenNotFound
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

43) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testAcceptExpiredToken
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

44) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testDecline with data set "local attendee" (false)
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

45) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testDecline with data set "external attendee" (true)
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

46) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testProcessMoreOptionsResult with data set "local attendee" (false)
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

47) OCA\DAV\Tests\Unit\DAV\Controller\InvitationResponseControllerTest::testProcessMoreOptionsResult with data set "external attendee" (true)
Method eq may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

48) OCA\DAV\Tests\unit\CalDAV\CalendarImplTest::testHandleImipMessage
Trying to configure method "getPlugin" which cannot be configured because it does not exist, has not been specified, is final, or is static

49) OCA\DAV\Tests\unit\CalDAV\CalendarImplTest::testHandleImipMessageNoCalendarUri
Trying to configure method "getPlugin" which cannot be configured because it does not exist, has not been specified, is final, or is static

50) OCA\DAV\Tests\unit\BackgroundJob\CleanupInvitationTokenJobTest::testRun
Method lt may not return value of type Mock_IQueryFunction_d2e3ec53, its return declaration is ": string"

@juliusknorr
Copy link
Member Author

@miaulalala Any chance you could have a look at the failing ones in OCA\DAV\Tests\unit\CalDAV\CalendarImplTest I gave it a quick attempt by mocking the calls on the getPlugin method and adding the class to the DB group (as InvitationResponseServer pulls in IDBConnection through DI) but then it fails with the following error:

Sabre\VObject\ParseException : Invalid Mimedir file. Line starting at 11 did not follow iCalendar/vCard conventions

@juliusknorr
Copy link
Member Author

@PVince81 It seems the tests from #32690 were already causing warnings on the original PR. Do you remember if they passed locally? I tried running them with the files_external app enabled and added them to the DB group but they still do not pass, maybe you have a quick idea :)

@juliusknorr juliusknorr added 2. developing Work in progress tests Related to tests labels Oct 26, 2022
@miaulalala
Copy link
Contributor

@juliushaertl Not sure if that did the trick. Removed some superfluous text from the VEvent.

@juliusknorr
Copy link
Member Author

@miaulalala I pushed my attempts to fix but tbh I have no idea what this is testing 🙈 I've added some fixme statements to where I think you might be able to tell with the Imip knowledge.

The previous attempt to mock the method calls didn't work so I refactored that a bit so the individual getPlugin methods could be mocked, but the mocking seems quite extensive but I currently don't see a better way.

@miaulalala
Copy link
Contributor

@miaulalala I pushed my attempts to fix but tbh I have no idea what this is testing see_no_evil I've added some fixme statements to where I think you might be able to tell with the Imip knowledge.

The previous attempt to mock the method calls didn't work so I refactored that a bit so the individual getPlugin methods could be mocked, but the mocking seems quite extensive but I currently don't see a better way.

I changed some bits, not sure if it did the trick as I keep having issues with running tests locally. Let's see what CI says.

@juliusknorr
Copy link
Member Author

I managed to make them pass, though I'm not really sure if the adjustments make sense. Maybe you can double check that @miaulalala

Besides that after the rebase there are more calendar related warnings (which means the tests don't run):

1) Test\Calendar\ManagerTest::testHandleImipReplyEventNotFound
227	Trying to configure method "handleIMipMessage" which cannot be configured because it does not exist, has not been specified, is final, or is static
228	
229	2) Test\Calendar\ManagerTest::testHandleImipReply
230	Trying to configure method "handleIMipMessage" which cannot be configured because it does not exist, has not been specified, is final, or is static
231	
232	3) Test\Calendar\ManagerTest::testHandleImipCancelOrganiserInReplyTo
233	Trying to configure method "handleIMipMessage" which cannot be configured because it does not exist, has not been specified, is final, or is static
234	
235	4) Test\Calendar\ManagerTest::testHandleImipCancel
236	Trying to configure method "handleIMipMessage" which cannot be configured because it does not exist, has not been specified, is final, or is static
237	

Running them locally should be just a matter of running this command on an active development instance:

phpunit -c tests/phpunit-autotest.xml apps/dav/tests/unit/CalDAV/CalendarImplTest.php

@miaulalala
Copy link
Contributor

miaulalala commented Nov 8, 2022

I think the warnings are from the move to the separate IHandleImipMessage interface. Can you rebase?

No it shouldn't be, as your branch still has the old interface...

@miaulalala
Copy link
Contributor

My tests now pass locally. Do yours?

@juliusknorr juliusknorr force-pushed the tests/fix-phpunit-warnings branch from b001804 to e29720f Compare November 9, 2022 15:37
@juliusknorr
Copy link
Member Author

Thanks, yep seems to all pass now :) Thanks for your help

This was referenced Nov 14, 2022
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see this merged for #32595
Are there known blockers?

apps/dav/lib/CalDAV/CalendarImpl.php Outdated Show resolved Hide resolved
@juliusknorr
Copy link
Member Author

I would like to see this merged for #32595
Are there known blockers?

I don't have a good idea how to adapt Test\Encryption\UtilTest. Maybe @PVince81 or @icewind1991 can help with that as most of the tested logic seems to have been moved out with #33500

Other than that no blocker from my side

@PVince81
Copy link
Member

@juliushaertl I don't remember the details but I do remember struggling with the fact that we use GlobalStoragesService in the test while the app might not be enabled for all tests. I vaguely remember that I might have just enabled the files_external app always in enable.php as a workaround.

if that test is the only issue I suggest we handle this separately so we can merge this PR ?

@come-nc come-nc self-assigned this Dec 5, 2022
Signed-off-by: Côme Chilliet <[email protected]>
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I applied my suggestions, good to merge for me.
Test\Encryption\UtilTest will be fixed later in an other PR.

@come-nc come-nc added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 5, 2022
Signed-off-by: Côme Chilliet <[email protected]>
@come-nc
Copy link
Contributor

come-nc commented Dec 5, 2022

There were 2 errors:

1) OCA\DAV\Tests\unit\CalDAV\CalendarImplTest::testHandleImipMessage
PHPUnit\Framework\MockObject\IncompatibleReturnValueException: Method getServer may not return value of type Mock_Server_5e1436fe, its declared return type is "OCA\DAV\Connector\Sabre\Server"

/home/runner/work/server/server/apps/dav/tests/unit/CalDAV/CalendarImplTest.php:188

2) OCA\DAV\Tests\unit\CalDAV\CalendarImplTest::testHandleImipMessageNoCalendarUri
PHPUnit\Framework\MockObject\IncompatibleReturnValueException: Method getServer may not return value of type Mock_Server_5e1436fe, its declared return type is "OCA\DAV\Connector\Sabre\Server"

/home/runner/work/server/server/apps/dav/tests/unit/CalDAV/CalendarImplTest.php:232

@come-nc come-nc enabled auto-merge December 5, 2022 17:25
@come-nc come-nc disabled auto-merge December 6, 2022 09:51
@come-nc come-nc merged commit 67ff8fc into master Dec 6, 2022
@come-nc come-nc deleted the tests/fix-phpunit-warnings branch December 6, 2022 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews tests Related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants