[Storage] Download file test #2220
Conversation
juans-chainsafe
added
the
Type: Maintenance
|
Your Render PR Server URL is https://chainsafe-components-stage-pr-2220.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-cb656kkobjd830ivjelg. |
|
Your Render PR Server URL is https://files-ui-stage-pr-2220.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-cb656lsobjd830ivjeq0. |
|
Your Render PR Server URL is https://storage-ui-stage-pr-2220.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-cb656n4objd830ivjfbg. |
| @@ -143,7 +144,6 @@ describe("Bucket management", () => { | |||
| bucketsPage.bucketItemName().eq(0).should("have.text", chainSafeBucketName) | |||
| bucketsPage.bucketItemName().eq(1).should("have.text", ipfsBucketName) | |||
| }) | |||
|
|
|||
| // upload a file and store file content | ||
| navigationMenu.bucketsNavButton().click() | ||
|
|
||
| bucketsPage.createBucket(chainSafeBucketName, "chainsafe") |
There was a problem hiding this comment.
Something for the future / separate ticket perhaps but when we need to create buckets as requisites for tests (but we're not testing bucket creation itself) it would probably be better to expand the apiTestHelper to add this ability. This way we can create the bucket programmatically and have it there at the beginning of the test which would be more efficient than setting it up in the UI.
Our FE developers would be best able to advise on the feasibility of doing this but I think it should be possible.
There was a problem hiding this comment.
I think I can try to do this now, sounds fun
There was a problem hiding this comment.
Its definitely doable. Shout if you have any issues @juans-chainsafe
| cy.wait("@downloadRequest").should((download) => { | ||
| expect(download.response).to.have.property("statusCode", 200) | ||
| }) | ||
| }) |
asnaith
dismissed
their stale review
Additional changes coming for apiTestHelper
| navigationMenu.cidsNavButton().click() | ||
| navigationMenu.bucketsNavButton().click() | ||
| bucketsPage.awaitBucketRefresh() | ||
| bucketsPage.bucketItemRow().should("have.length", 1) |
There was a problem hiding this comment.
The navigation clicks that move away and then back to the buckets page are just to ensure that the UI is updated after creating the required buckets via the api, so I don't think we should be doing this before resolve()?
We should also catch any errors that happen during bucket creation so that we know where the problem is if/when it occurs. Otherwise, the problem would be obscured and it will result in a failure of the next test step (which it shouldn't have got to) and this could cause confusion when debugging failing tests.
If we do this, we can then we can omit bucketsPage.bucketItemRow().should("have.length", 1) which I think makes sense, we shouldn't be using assertions outside of the test layer and this would also fail if more than one bucket was created anyway.
There was a problem hiding this comment.
You are right that should be after the resolve, buuut we need that because we are using the api to create the bucket, so the UI is not updated with that bucket without refreshing the page (I change the go to cids and then go to buckets, with a cy.refresh() I think has more sence).
Added as well the try and catch logic and removed the validation
There was a problem hiding this comment.
Sounds good. We have seen "detached from DOM" issues when using cy.reload() in the past but if it's working reliably here then that's fine with me :)
There was a problem hiding this comment.
I will take a look at that, and if happens, I will go to the original implementation of going to cids and then bucket page
| @@ -186,11 +183,10 @@ describe("Bucket management", () => { | |||
| const folderName = `folder ${Date.now()}` | |||
| const newFolderName = `new folder name ${Date.now()}` | |||
|
|
|||
| cy.web3Login({ deleteFpsBuckets: true }) | |||
| cy.web3Login({ deleteFpsBuckets: true, createFpsBuckets: [{ name: chainsafeBucketName, type: "chainsafe" }] }) | |||
| navigationMenu.bucketsNavButton().click() | |||
|
|
|||
| // create a new bucket and go inside the bucket | |||
There was a problem hiding this comment.
This comment can be removed now as the bucket is already created by this point
There was a problem hiding this comment.
Thanks for this @juans-chainsafe!
I think it would be a good idea to get a second review from @FSM1 on this before merging because these apiTestHelper changes arrived after the original approval.
Checked through the bucket creation code and it looks good to me. Happy with this being merged @juans-chainsafe @asnaith |
closes #2205