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

Make integration tests work with both PHP 7.3 and 7.4 #24895

Merged

Conversation

danxuliu
Copy link
Member

@danxuliu danxuliu commented Dec 29, 2020

Found by @juliushaertl in #24551

The Trashbin and WebDav traits were using each other in a circular dependency (WebDav -> Sharing -> Provisioning -> BasicStructure -> Trashbin -> WebDav). In PHP 7.3 this worked fine, but in PHP 7.4 the fatal error Trait 'WebDav' not found in .../Trashbin.php was thrown. To solve this now the TrashBin trait no longer explicitly uses WebDav.

However, due to this change, the class using TrashBin is now expected to also use WebDav. As the Trashbin trait was not needed by most contexts using the BasicStructure trait Trashbin was removed from it and added only to those contexts that actually need it.

I will keep this in developing state until CI has finished just in case there is some other context that needs to use the Trashbin trait too (it helps not to forget to commit the changes to check them... 🤦).

Unfortunately I do not know if the issue is in PHP 7.4 itself, Behat, its autoloader... But I have distilled the issue to the following test case:
integration-tests-recursive-traits.patch.txt

After applying the patch,

  • ./run-docker.sh --image nextcloudci/php7.3:php7.3-5 features/traittest.feature 👍 works
  • ./run-docker.sh --image nextcloudci/php7.4:php7.4-3 features/traittest.feature 💥 shows PHP Fatal error: Trait 'TraitTestA' not found in /nextcloud/build/integration/features/bootstrap/TraitTestB.php on line 5

Therefore it seems that the problem is in the circular dependency between the traits.

The "Trashbin" and "WebDav" traits were using each other in a circular
dependency ("WebDav" -> "Sharing" -> "Provisioning" -> "BasicStructure"
-> "Trashbin" -> "WebDav"). In PHP 7.3 this worked fine, but in PHP 7.4
the fatal error "Trait 'WebDav' not found in .../Trashbin.php" was
thrown. To solve this now the "TrashBin" trait no longer explicitly uses
"WebDav".

However, due to this change, the class using "TrashBin" is now expected
to also use "WebDav". As the "Trashbin" trait was not needed by most
contexts using the "BasicStructure" trait "Trashbin" was removed from it
and added only to those contexts that actually need it.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the make-integration-tests-work-with-both-php-7.3-and-7.4 branch from 1a23006 to 28f2d0e Compare December 30, 2020 05:02
@danxuliu danxuliu added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 30, 2020
@danxuliu danxuliu marked this pull request as ready for review December 30, 2020 05:03
@faily-bot
Copy link

faily-bot bot commented Dec 30, 2020

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 833: failure

acceptance-app-files-sharing

  • tests/acceptance/features/app-files-sharing.feature:23
Show full log
  Scenario: share a file with another user that needs to accept shares        # /drone/src/tests/acceptance/features/app-files-sharing.feature:23
    Given I act as John                                                       # ActorContext::iActAs()
    And I am logged in as the admin                                           # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I act as Jane                                                         # ActorContext::iActAs()
    And I am logged in                                                        # LoginPageContext::iAmLoggedIn()
    And I visit the settings page                                             # SettingsMenuContext::iVisitTheSettingsPage()
    And I open the "Sharing" section                                          # AppNavigationContext::iOpenTheSection()
    And I disable accepting the shares by default                             # SettingsContext::iDisableAcceptingTheSharesByDefault()
    And I see that shares are not accepted by default                         # SettingsContext::iSeeThatSharesAreNotAcceptedByDefault()
    And I act as John                                                         # ActorContext::iActAs()
    And I rename "welcome.txt" to "farewell.txt"                              # FileListContext::iRenameTo()
    And I see that the file list contains a file named "farewell.txt"         # FileListContext::iSeeThatTheFileListContainsAFileNamed()
    When I share "farewell.txt" with "user0"                                  # FilesAppSharingContext::iShareWith()
    And I see that the file is shared with "user0"                            # FilesAppSharingContext::iSeeThatTheFileIsSharedWith()
    And I act as Jane                                                         # ActorContext::iActAs()
    And I open the Files app                                                  # FilesAppContext::iOpenTheFilesApp()
    And I see that the file list does not contain a file named "farewell.txt" # FileListContext::iSeeThatTheFileListDoesNotContainAFileNamed()
    And I accept the share for "/farewell.txt" in the notifications           # NotificationsContext::iAcceptTheShareForInTheNotifications()
      Notifications button in the header could not be found after 100 seconds (NoSuchElementException)
    And I open the Files app                                                  # FilesAppContext::iOpenTheFilesApp()
    Then I see that the file list contains a file named "farewell.txt"        # FileListContext::iSeeThatTheFileListContainsAFileNamed()
    And I open the details view for "farewell.txt"                            # FileListContext::iOpenTheDetailsViewFor()
    And I see that the details view is open                                   # FilesAppContext::iSeeThatTheDetailsViewIsOpen()
    And I open the "Sharing" tab in the details view                          # FilesAppContext::iOpenTheTabInTheDetailsView()
    And I see that the "Sharing" tab in the details view is eventually loaded # FilesAppContext::iSeeThatTheTabInTheDetailsViewIsEventuallyLoaded()
    And I see that the file is shared with me by "admin"                      # FilesAppSharingContext::iSeeThatTheFileIsSharedWithMeBy()

@rullzer rullzer mentioned this pull request Dec 30, 2020
39 tasks
@rullzer rullzer merged commit 2ac0e89 into master Dec 30, 2020
@rullzer rullzer deleted the make-integration-tests-work-with-both-php-7.3-and-7.4 branch December 30, 2020 09:52
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 bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants