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

Improve bucket upload ui tests #2117

Merged
merged 15 commits into from
May 10, 2022
Merged

Conversation

asnaith
Copy link
Member

@asnaith asnaith commented May 4, 2022

closes #2085

Our automation tests in this area were pretty basic, only creating and deleting the default bucket. If bucket uploading was broken for a specific file system type then we couldn't detect it.

These updated tests now provide more robust coverage for the bucket create, upload, and delete flow and utilize both file system types, ChainSafe and IPFS.

@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 May 4, 2022
@render
Copy link

render bot commented May 4, 2022

@render
Copy link

render bot commented May 4, 2022

@render
Copy link

render bot commented May 4, 2022

@@ -1,4 +1,5 @@
export const bucketName = "test bucket"
export const chainSafeBucketName = `cs bucket ${Date.now()}`
export const ipfsBucketName = `ipfs bucket ${Date.now()}`
Copy link
Member Author

Choose a reason for hiding this comment

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

We remove the bucket from the UI instantly and don't wait for any confirmation so to avoid issues I'm using unique names here.

...basePage,

// bucket content browser elements
bucketHeaderLabel: () => cy.get("[data-cy=header-bucket]", { timeout: 20000 }),
Copy link
Contributor

Choose a reason for hiding this comment

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

this timeout is because this type of web elements take longer to load than usual, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually...this was unnecessary 😄. It was left there by accident. I have pushed a commit to remove it and some others which aren't needed.

The default is 6 seconds and that will be ok.

@@ -136,7 +136,7 @@ const UploadFileModal = ({ modalOpen, close }: IUploadFileModuleProps) => {
/>
<footer className={classes.footer}>
<Button
data-cy="upload-cancel-button"
testId="cancel-upload"
Copy link
Contributor

Choose a reason for hiding this comment

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

why testId and not data-cy? this modal es reusable?

Copy link
Member Author

Choose a reason for hiding this comment

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

@juans-chainsafe In this example specifically, the button component already has a testId prop so I'm just passing something through to that, as opposed to writing a custom attribute directly on the element. It's a little cleaner / more readable ("testId" is instantly obvious to all reading the code what it's for).

Screen Shot 2022-05-04 at 10 06 30 AM

For buttons, either type of locator will work, but there are instances with other components where this is not the case:

  • Due to the manner in which we are setting up the element in code (maybe using the default component without further customizations) it's sometimes hard to add a custom data-cy attribute to it (off the top of my head I think toasts in Files is a good example).

  • I've also seen instances where even though we have set up a data-cy tag Cypress hasn't been able to find it but by setting up a testId on the component and passing something through to it does work (saw this recently with radio buttons).

I am certain you will encounter this too at some point. Hopefully, this explanation makes sense but we can also discuss it further if you want :)

Copy link
Contributor

@juans-chainsafe juans-chainsafe left a comment

Choose a reason for hiding this comment

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

Looks great, you had a lot of work here! awesome

@asnaith asnaith requested a review from tanmoyAtb May 10, 2022 03:44
Copy link
Contributor

@tanmoyAtb tanmoyAtb left a comment

Choose a reason for hiding this comment

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

Looks good to me !

@asnaith asnaith merged commit 0ae6827 into dev May 10, 2022
@asnaith asnaith deleted the mnt/expand-bucket-upload-tests-2085 branch May 10, 2022 18:58
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.

Expand ui tests for storage ui file upload
4 participants