-
-
Notifications
You must be signed in to change notification settings - Fork 89
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
Split command to 'attachFile' using 'cy.readFile' and 'attachFixture' using 'cy.fixture' #304
Conversation
@abramenal can you please review it? |
@abramenal can you please check this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed explanation this. I really don't see why this wasn't yet fixed internally by cypress.
Apologies for this shit timing, let's get this delivered this week
@@ -0,0 +1,10 @@ | |||
import { wrapBlob } from './common'; | |||
|
|||
export default function getFileContent({ filePath, fileContent, fileEncoding }) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Function name should match the file name
``` | ||
|
||
It is a common practice to put all the files required for Cypress tests inside `cypress/fixtures` folder and call them as fixtures (or a fixture). The command recognizes [`cy.fixture`][cy.fixture] format, so usually this is just a file name. | ||
It is a common practice to put all the files required for Cypress tests inside `cypress/fixtures` folder and call them as fixtures (or a fixture). `attachFixture` command recognizes [`cy.fixture`][cy.fixture] format, so usually this is just a file name. `attachFile` on the other hand recognise [`cy.readFile`][cy.readFile], so full file path is needed. `optionalProcessingConfig` is the same for both commands. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a common practice to put all the files required for Cypress tests inside `cypress/fixtures` folder and call them as fixtures (or a fixture). `attachFixture` command recognizes [`cy.fixture`][cy.fixture] format, so usually this is just a file name. `attachFile` on the other hand recognise [`cy.readFile`][cy.readFile], so full file path is needed. `optionalProcessingConfig` is the same for both commands. | |
It is a common practice to put all the files required for Cypress tests inside `cypress/fixtures` folder and call them as fixtures (or a fixture). `attachFixture` command recognizes [`cy.fixture`][cy.fixture] format, so usually this is just a file name. `attachFile` on the other hand recognise [`cy.readFile`][cy.readFile], so a path within the project root folder is needed. `optionalProcessingConfig` is the same for both commands. |
Copying this from cy.readFile docs. First got confused as thought it can also accept global file path
@all-contributors add @ErikGrigoriev for code |
I've put up a pull request to add @ErikGrigoriev! 🎉 |
@ErikGrigoriev one thing I am a bit unsure of is – whether to allow raw file contents to be passed. With this kind of separation it might make sense to create a separate command for that, so:
|
Checklist:
Summary of changes
Added possibility to upload file instead of fixture. Split to 2 different commands. Renamed actual
attachFile
toattachFixture
and reworkedattachFile
to usecy.readFile
instead. Would force existing users to rename all their usages toattachFixture
in order to use it as before, but according to new functionality this naming seems to be more precise.Linked issues
Closes #232
Comment
Main reason for introducing this is bug this bug from cypress itself. Using fixture for upload failed in our project since we were creating files with same name for upload. And even though files were deleted and recreated between tests, cached data was used causing upload inconsistency.