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]Add valid tests that were removed by PR #5973 #6047

Merged
merged 2 commits into from
Dec 2, 2021

Conversation

SwikritiT
Copy link
Contributor

@SwikritiT SwikritiT commented Nov 23, 2021

Description

Some of the tests that are still valid were accidentally removed by PR #5973. This PR re-adds those tests. One of the added tests relating to versions was flaky so a little pause time is added and the function is refactored accordingly

Related Issue

Motivation and Context

How Has This Been Tested?

  • Locally
  • CI

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • ...

@SwikritiT SwikritiT self-assigned this Nov 23, 2021
@SwikritiT SwikritiT force-pushed the restore-deleted-tests branch from 3fe76aa to a29312e Compare November 23, 2021 08:39
@SwikritiT SwikritiT changed the title [tests-only][full-ci]Add valid tests that were removed by PR [tests-only][full-ci]Add valid tests that were removed by PR #5973 Nov 23, 2021
@@ -85,3 +85,14 @@ Feature: Versions of a file
And the user uploads overwriting file "lorem.txt" using the webUI
And the user browses to display the "versions" details of file "lorem.txt"
Then the versions list should contain 1 entries

@issue-ocis-1328 @disablePreviews
Scenario: sharee can see the versions of a file
Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty sure this one was deleted as flaky(?) also failes CI in this PR

Copy link
Contributor Author

@SwikritiT SwikritiT Nov 24, 2021

Choose a reason for hiding this comment

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

Yes. It was skipped previously because of its flakiness but should it be entirely removed? I'll try to find out if it's the test timing problem or the backend issue. if it's the latter we can skip it again and probably create an issue

@SwikritiT SwikritiT force-pushed the restore-deleted-tests branch 2 times, most recently from 43bc873 to e28f253 Compare November 29, 2021 06:51
@SwikritiT
Copy link
Contributor Author

After refactoring the function and making it similar to the delete function

exports.delete = async function (userId, file) {
(which also has a similar issue of timing) and increasing the wait time the test is passing even in multiple runs https://drone.owncloud.com/owncloud/web/20462/9/19. We can decide if we want to keep these changes and increase the time or not. If not the tests will have to be skipped in the oc10 backend

@SwikritiT SwikritiT marked this pull request as ready for review November 29, 2021 08:04
@SwikritiT SwikritiT requested review from saw-jan, dpakach and Talank and removed request for kulmann and fschade November 29, 2021 08:04
Comment on lines +204 to +205
if (timeSinceLastFileUpload <= 1600) {
await client.pause(1600 - timeSinceLastFileUpload)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adjusting this pause time to 1600 fixes the versions issue, the tests are passing even in multiple runs https://drone.owncloud.com/owncloud/web/20534/9/19

@fschade
Copy link
Contributor

fschade commented Nov 30, 2021

I excluded the oc10 test yesterday, can you please re enable it once this is stable? Thanks 😊

@disablePreviews @skipOnOC10 @issue-5853

@SwikritiT SwikritiT force-pushed the restore-deleted-tests branch from c55c022 to b5beb4a Compare December 1, 2021 08:16
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 1, 2021

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

@ownclouders
Copy link
Contributor

Results for oC10SharingPubExpAndRoles https://drone.owncloud.com/owncloud/web/20576/39/1
The following scenarios passed on retry:

  • webUISharingPublicExpire/shareByPublicLinkExpiringLinks.feature:11

@phil-davis phil-davis merged commit 0bf39e8 into master Dec 2, 2021
@delete-merged-branch delete-merged-branch bot deleted the restore-deleted-tests branch December 2, 2021 06:41
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.

6 participants