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] Adds e2e tests for download files batch action #6541

Merged
merged 3 commits into from
Apr 29, 2022

Conversation

SwikritiT
Copy link
Contributor

@SwikritiT SwikritiT commented Mar 7, 2022

Description

Adds test implementation for step Alice downloads 2 files as tar.

Related Issue

Motivation and Context

How Has This Been Tested?

  • local
  • 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
  • e2e tests added
  • Documentation ticket raised:

Open tasks:

  • ...

@SwikritiT SwikritiT changed the base branch from master to unauthorizedUserOpenPublicLink March 7, 2022 09:02
@SwikritiT SwikritiT force-pushed the downloadPubicLinkFiles branch 2 times, most recently from 05ecaae to f40d238 Compare March 7, 2022 09:38
@ownclouders
Copy link
Contributor

Results for e2e-tests oC10 https://drone.owncloud.com/owncloud/web/23335/10/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/public/owncloud/web/23335/tracing/alice-creates-public-link-to-a-folder-alice-2022-3-7-09-54-44.zip

@SwikritiT SwikritiT self-assigned this Mar 7, 2022
@SwikritiT SwikritiT mentioned this pull request Mar 7, 2022
13 tasks
@SwikritiT
Copy link
Contributor Author

The base branch needs to be merged first #6461

@SwikritiT SwikritiT marked this pull request as ready for review March 9, 2022 04:08
Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

Please don't merge before @fschade has brought in his E2E improvements

@amrita-shrestha amrita-shrestha force-pushed the unauthorizedUserOpenPublicLink branch 2 times, most recently from 103143e to 62bf306 Compare March 10, 2022 03:17
@SwikritiT SwikritiT force-pushed the downloadPubicLinkFiles branch from 6d46892 to 5c9ea1a Compare March 10, 2022 06:19
@SwikritiT
Copy link
Contributor Author

Please don't merge before @fschade has brought in his E2E improvements

@pascalwengerter Is it limited to this PR only? There is another PR too for the e2e tests #6461 that's ready to be merged. So let us know if we need to withhold that too.
Also, just out of curiosity what sort of improvements are we talking about?

@delete-merged-branch delete-merged-branch bot deleted the branch master March 14, 2022 03:55
@SwikritiT SwikritiT changed the base branch from unauthorizedUserOpenPublicLink to master March 15, 2022 03:46
@SwikritiT SwikritiT force-pushed the downloadPubicLinkFiles branch 2 times, most recently from 0e3d8d4 to 98e5ffc Compare April 5, 2022 03:59
@fschade
Copy link
Contributor

fschade commented Apr 7, 2022

is this still active or should we take over?

@SwikritiT
Copy link
Contributor Author

SwikritiT commented Apr 8, 2022

is this still active or should we take over?

I was actually waiting for you to review it again. I have addressed your previous reviews. Can you take a look at it again?

@SwikritiT SwikritiT force-pushed the downloadPubicLinkFiles branch 3 times, most recently from fccfcc6 to b4e626f Compare April 12, 2022 03:27
@SwikritiT SwikritiT changed the title [tests-only] Download files batch action [tests-only] Adds e2e tests for download files batch action Apr 12, 2022
Copy link
Contributor

@fschade fschade left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, just some minor nitpicks.

folder: string
}

export const selectMultipleResources = async (args: selectResourcesArgs): Promise<void> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

as reference for later, if we have selectRsources we also need a way to reset/deselect them again if a action on the same path should happen. No action item.

@SwikritiT SwikritiT force-pushed the downloadPubicLinkFiles branch 2 times, most recently from 95d7e9f to 417e902 Compare April 13, 2022 04:02
@SwikritiT SwikritiT requested a review from individual-it April 13, 2022 04:06
@SwikritiT SwikritiT force-pushed the downloadPubicLinkFiles branch 2 times, most recently from b05f23d to 552d06b Compare April 19, 2022 03:53
@SwikritiT SwikritiT force-pushed the downloadPubicLinkFiles branch from 552d06b to 2172893 Compare April 20, 2022 04:22
@SwikritiT SwikritiT requested a review from fschade April 20, 2022 05:20
@SwikritiT
Copy link
Contributor Author

@fschade I've addressed your reviews can you take a look?

@SwikritiT SwikritiT force-pushed the downloadPubicLinkFiles branch from 175e544 to ccd6acf Compare April 25, 2022 04:02
@SwikritiT SwikritiT force-pushed the downloadPubicLinkFiles branch from ccd6acf to ad36f30 Compare April 29, 2022 03:55
@SwikritiT SwikritiT force-pushed the downloadPubicLinkFiles branch from a72fb45 to 4db25e8 Compare April 29, 2022 05:41
@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
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for pushing through here!

@@ -0,0 +1,35 @@
Feature: link

@issue-6239
Copy link
Contributor

Choose a reason for hiding this comment

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

The linked issue has been resolved so after this PR has been merged we should be able to run the directory download tests for both backends (= remove the oc10 from the filename)

@pascalwengerter
Copy link
Contributor

Also please use the squash-merge option here:)

Copy link
Contributor

@fschade fschade left a comment

Choose a reason for hiding this comment

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

LGTM 🎸

@pascalwengerter pascalwengerter merged commit efc2864 into master Apr 29, 2022
@delete-merged-branch delete-merged-branch bot deleted the downloadPubicLinkFiles branch April 29, 2022 09:26
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.

8 participants