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

Add ui tests for copying / moving to share folders #2032

Merged
merged 11 commits into from
Mar 22, 2022

Conversation

asnaith
Copy link
Member

@asnaith asnaith commented Mar 22, 2022

closes #2029

adds coverage for the 4 scenarios when copying / moving files to new / existing shares and preserving / deleting the original

@asnaith asnaith added the Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes. label Mar 22, 2022
@asnaith asnaith requested review from tanmoyAtb, FSM1 and Tbaut March 22, 2022 01:55
@render
Copy link

render bot commented Mar 22, 2022

@render
Copy link

render bot commented Mar 22, 2022

@render
Copy link

render bot commented Mar 22, 2022

Copy link
Contributor

@FSM1 FSM1 left a comment

Choose a reason for hiding this comment

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

great stuff 🥇

Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Awesome! I left a couple nit suggestions

packages/files-ui/cypress/tests/file-sharing-spec.ts Outdated Show resolved Hide resolved
packages/files-ui/cypress/tests/file-sharing-spec.ts Outdated Show resolved Hide resolved
packages/files-ui/cypress/tests/file-sharing-spec.ts Outdated Show resolved Hide resolved
packages/files-ui/cypress/tests/file-sharing-spec.ts Outdated Show resolved Hide resolved
packages/files-ui/cypress/tests/file-sharing-spec.ts Outdated Show resolved Hide resolved
packages/files-ui/cypress/tests/file-sharing-spec.ts Outdated Show resolved Hide resolved
packages/files-ui/cypress/tests/file-sharing-spec.ts Outdated Show resolved Hide resolved
packages/files-ui/cypress/tests/file-sharing-spec.ts Outdated Show resolved Hide resolved
Comment on lines 316 to 317
cy.get("@fileName").then(($fileName) => {
sharedPage.fileItemName().contains(`${$fileName}`).should("be.visible")
Copy link
Collaborator

Choose a reason for hiding this comment

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

A little more elegant IMHO. Feel free to ignore if you feel it's too complex

Suggested change
cy.get("@fileName").then(($fileName) => {
sharedPage.fileItemName().contains(`${$fileName}`).should("be.visible")
cy.get<string>("@fileName").then((fileName) => {
sharedPage.fileItemName().contains(fileName).should("be.visible")

Copy link
Member Author

Choose a reason for hiding this comment

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

@Tbaut I like it. Thanks for the suggestion, I'm always thankful and appreciative of these tips to write cleaner code.

I've gone ahead and used it as suggested and I also updated all the other places where we are doing similar things. It looks and reads better 👍

@asnaith asnaith merged commit 42920f1 into dev Mar 22, 2022
@asnaith asnaith deleted the mnt/add-share-copy-tests-2029 branch March 22, 2022 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Maintenance Added to issues and PRs when a change is for repository maintenance , such as CI or linter changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ui test coverage for file copying to share folder
3 participants