elgentos / magento2-cypress-testing-suite

A community-driven Cypress testing suite for Magento 2
MIT License
172 stars 37 forks source link

Update the config to delete the recorded video if no tests are retried #98

Closed shyambh closed 2 years ago

shyambh commented 2 years ago

I have updated the cypress.config.js file to add the code to delete the recorded videos if none of the tests are retried. I basically simply followed the steps mentioned here: https://docs.cypress.io/api/plugins/after-spec-api#Delete-the-recorded-video-if-no-tests-retried

shyambh commented 2 years ago

Also can you please add 'hactoberfest-accepted' label to this PR if you think it is fine?

Like so: image

peterjaap commented 2 years ago

@shyambh I didn't know I had to do that, sorry. Done now.

Vinai commented 2 years ago

This MR breaks the config with

Your configFile is invalid: ...magento2-cypress-testing-suite/cypress.config.js

It threw an error when required, check the stack trace below:

Error [ERR_REQUIRE_ESM]: require() of ES Module ...magento2-cypress-testing-suite/node_modules/del/index.js from ...magento2-cypress-testing-suite/cypress.config.js not supported.
Instead change the require of index.js in /Volumes/CaseSensitive/Workspace/php-sites/mos-hyva-245/magento2-cypress-testing-suite/cypress.config.js to a dynamic import() which is available in all CommonJS modules.
Vinai commented 2 years ago

Adding a dependency for something as simple as deleting files and folders recursively is maybe not necessary.

shyambh commented 2 years ago

@Vinai Thank you for this. I will take a look at this.

Vinai commented 2 years ago

Another issue is you use _ (lowbar/underscore), which is not available.

return _.some(test.attempts, { state: 'failed' })

Again, instead of using a dependency, why not use native JS?

return test.attempts.find(({state}) => state === 'failed')

Vinai commented 2 years ago

Did you even test the code before submitting the PR @shyambh ?

shyambh commented 2 years ago

@Vinai I really want to apologize for this.

I used a dynamic import for the 'del' package and now the tests are running.

image

Regarding the dependency on the external package, I followed the implementation steps mentioned in the Cypress documentation here:

https://docs.cypress.io/api/plugins/after-spec-api#Delete-the-recorded-video-if-no-tests-retried

Please let me know if I can push the fix?

Vinai commented 2 years ago

Don't worry, already opened a PR https://github.com/elgentos/magento2-cypress-testing-suite/pull/100