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

Confusing behaviour when downloading files (Firefox automatically renames downloaded files and "trashAssetsBeforeRuns" only affects "run" mode) #22843

Open
mirobo opened this issue Jul 19, 2022 · 2 comments
Labels
E2E Issue related to end-to-end testing topic: downloads ⬇️ Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. type: enhancement Requested enhancement of existing feature

Comments

@mirobo
Copy link
Contributor

mirobo commented Jul 19, 2022

What would you like?

According to #5027 I quote
You can now test file downloads in Cypress without the download prompt displaying. Any files downloaded while testing file downloads will be stored in the downloadsFolder which is set to cypress/downloads by default. The downloadsFolder will be deleted before each run unless trashAssetsBeforeRuns is set to false. This means that if you had any code written to prevent the download prompt or to configure the download location (like in a before:browser:launch event handler), you can remove all of these workarounds

There are some inconveniences and pitfalls which results in discussions with developers and unexpected behaviours (at first I wanted to create an issue for that)

When developing/running tests locally in "open" mode the parameter "trashAssetsBeforeRuns" will not do anything therefor NOT remove previously downloaded files (this is working as documented but IMO wildly unexpected!). This leads to the behaviour that Firefox renames the file to "myFilename(1)", "myFilename(2)" and so on. Any assertion with cy.readFile('myFilename') in "open" mode will pass, because the previously downloaded file "myFilename" still exists. Cypress does the assertion on the wrong file. You could locally break your app and not see the error in the local Cypress run in "open" mode. Only in CI you will detect the error.

Cypress also knows that the downloaded file was renamed:
image

Reading a downloaded file and asserting the filename should not be the same thing. In some cases it may be irrelevant what the exact file name is (because it may be generated or some random suffix is added). Or sometimes you only care about that at least a file was downloaded and the content is verified.

"trashAssetsBeforeRuns" should also clean the assets before a test execution in "open" mode. Then the issue with Firefox will disappear as there (most likely) won't be any renamed files with "(1)" attached. And it is more likely that the downloaded file is actually the file we are running assertions on.

Or it could be solved like this: It should be possible to fetch the information from the Cypress "download" event so we can get the actual filename, open it (no matter what has been "suffixed" by the browser) and run an assertion.
Possible usage of this function:

cy.readDownloadedFile().then((file) => {
  expect(file.name).to.match(/^myFilename.*\.txt$/);
  expect(file.content).to.eq('whatever');
});

or to address the possible issue that multiple downloads are triggered (in a random order) and we need to verify them all:

cy.readDownloadedFiles().then((files) => {
  // order of downloaded files unknown:
  const myFile = files.filter((f) => /myFilename/.test(f.name))[0];
  expect(myFile.name).to.match(/^myFilename.*\.txt$/);
  expect(myFile.content).to.eq('whatever');

  // order known:
  expect(files[0].name).to.match(/^myFilename.*\.txt$/);
  expect(files[0].content).to.eq('whatever');
  expect(files[1].name).to.match(/^otherFile.*\.txt$/);
  expect(files[1].content).to.eq('whatever');

});

Why is this needed?

  • Consistent behaviour of parameter "trashAssetsBeforeRuns" leads to less questions

  • Tests should NOT pass by accident in "open" mode when dealing with downloaded files

  • The following hook in support/index.ts is also triggered in "open" mode. So it would make sense that "trashAssetsBeforeRuns" will be executed.
    Cypress.on('test:before:run', () => { ... });

Other

No response

@cypress-bot cypress-bot bot added the stage: investigating Someone from Cypress is looking into this label Jul 19, 2022
@tbiethman
Copy link
Contributor

@mirobo thank you for the ticket. These are all good points. I too was a curious why trashAssetsBeforeRuns only applied to run vs. open mode. It appears to have been driven off of an early assumption that only Cypress screenshots/videos were going in that directory. If the downloadsFolder is being used for general browser downloads as well, it is likely worth revisiting this.

@cypress-bot cypress-bot bot added stage: fire watch and removed stage: investigating Someone from Cypress is looking into this labels Jul 20, 2022
@tbiethman tbiethman added type: enhancement Requested enhancement of existing feature E2E-core labels Jul 20, 2022
@mirobo
Copy link
Contributor Author

mirobo commented Jul 21, 2022

See also
trashAssetsBeforeRuns does not clear screenshots folder #18410
Cypress is not downloading files in the cypress/downloads folder on Firefox (Windows) #17896
Unable to set downloadFolder path from plugin file - options.preferences.default['download'] #21012

I just saw in this example https://github.com/cypress-io/cypress-example-recipes/blob/master/examples/testing-dom__download/cypress/e2e/location-href-spec.cy.js that it's possible to just intercept the download. But I think it would be way easier if there was a function "readDownloadedFile" or as mentioned automatic cleanup of download directory

@mschile mschile added triage and removed triage labels Aug 18, 2022
@nagash77 nagash77 added E2E Issue related to end-to-end testing and removed E2E-core labels Nov 8, 2022
@nagash77 nagash77 added Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. and removed routed-to-e2e labels Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E2E Issue related to end-to-end testing topic: downloads ⬇️ Triaged Issue has been routed to backlog. This is not a commitment to have it prioritized by the team. type: enhancement Requested enhancement of existing feature
Projects
None yet
Development

No branches or pull requests

6 participants