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

[tests-only][full-ci]Retry listing notifications #6839

Merged
merged 5 commits into from
Jul 24, 2023

Conversation

SwikritiT
Copy link
Contributor

@SwikritiT SwikritiT commented Jul 18, 2023

Part of: owncloud/QA#817

Sometimes tests try to get the notification before the server sends it, so the in this PR if the message is not set, then try to get the notification again. This will hopefully prevent error's like owncloud/QA#817 (comment)

Related error

apiAntivirus/antivirus.feature:294
apiAntivirus/antivirus.feature:296: https://drone.owncloud.com/owncloud/ocis/24228/56/7

  Scenario Outline: upload a file with virus smaller than the upload threshold                                             # /drone/src/tests/acceptance/features/apiAntivirus/antivirus.feature:283
    Given the config "ANTIVIRUS_MAX_SCAN_SIZE" has been set to "100"                                                       # OcisConfigContext::theConfigHasBeenSetTo()
    And using <dav-path-version> DAV path                                                                                  # FeatureContext::usingOldOrNewDavPath()
    When user "Alice" uploads file "filesForUpload/filesWithVirus/eicar.com" to "/aFileWithVirus.txt" using the WebDAV API # FeatureContext::userUploadsAFileTo()
    Then the HTTP status code should be "201"                                                                              # FeatureContext::thenTheHTTPStatusCodeShouldBe()
    And user "Alice" should get a notification with subject "Virus found" and message:                                     # NotificationContext::userShouldGetANotificationWithMessage()
      | message                                                                             |
      | Virus found in aFileWithVirus.txt. Upload not possible. Virus: Win.Test.EICAR_HDB-1 |
    And as "Alice" file "/aFileWithVirus.txt" should not exist                                                             # FeatureContext::asFileOrFolderShouldNotExist()

    Examples:
      | dav-path-version |
      | old              |
        Failed step: And user "Alice" should get a notification with subject "Virus found" and message:
        Notice: Undefined property: stdClass::$message in /drone/src/tests/acceptance/features/bootstrap/NotificationContext.php line 205
      | new              |
      | spaces           |
        Failed step: And user "Alice" should get a notification with subject "Virus found" and message:
        Notice: Undefined property: stdClass::$message in /drone/src/tests/acceptance/features/bootstrap/NotificationContext.php line 205

@SwikritiT SwikritiT self-assigned this Jul 18, 2023
@SwikritiT SwikritiT marked this pull request as ready for review July 18, 2023 10:59
@phil-davis
Copy link
Contributor

https://drone.owncloud.com/owncloud/ocis/24541/47/5

runsh: Total unexpected failed scenarios throughout the test run:
apiAccountsHashDifficulty/assignRole.feature:45

Might be an unrelated fail - I will restart.

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Looks reasonable - trying one more time after 1 second should be enough to prevent this problem.

@SwikritiT SwikritiT force-pushed the fix/antivirus-notification-error branch from 743fc34 to 0d2fdf6 Compare July 20, 2023 05:50
@SwikritiT SwikritiT requested review from SagarGi and phil-davis July 20, 2023 05:51
@phil-davis phil-davis self-requested a review July 20, 2023 09:20
@SwikritiT SwikritiT force-pushed the fix/antivirus-notification-error branch from 0d2fdf6 to 32d2ee5 Compare July 21, 2023 08:37
@SwikritiT SwikritiT requested a review from saw-jan July 21, 2023 08:37
@SwikritiT SwikritiT force-pushed the fix/antivirus-notification-error branch from 32d2ee5 to b52b5f2 Compare July 24, 2023 03:25
Copy link
Contributor

@amrita-shrestha amrita-shrestha left a comment

Choose a reason for hiding this comment

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

lgtm

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Member

@saw-jan saw-jan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@SagarGi SagarGi left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@SagarGi SagarGi merged commit fb7ba62 into master Jul 24, 2023
@delete-merged-branch delete-merged-branch bot deleted the fix/antivirus-notification-error branch July 24, 2023 05:46
ownclouders pushed a commit that referenced this pull request Jul 24, 2023
* Retry listing notifications

* use loop instaed

* use loop instaed

* throw exception

* use do while loop
SwikritiT added a commit that referenced this pull request Jul 24, 2023
* Retry listing notifications

* use loop instaed

* use loop instaed

* throw exception

* use do while loop
saw-jan pushed a commit that referenced this pull request Jul 24, 2023
* Retry listing notifications

* use loop instaed

* use loop instaed

* throw exception

* use do while loop
ownclouders pushed a commit that referenced this pull request Jul 24, 2023
* Retry listing notifications

* use loop instaed

* use loop instaed

* throw exception

* use do while loop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants