Add ability to configure ‘upload’ and ‘storage’ of videos separately when test suite passes/fails. #2522
Comments
|
@GrayedFox This already exists, but the docs are not clear. #2217 |
|
And for those who want to disable uploading entirely and still save failed spec videos? I don't think it makes a lot of sense to bundle this into a single config switch - I can certainly see a use case where a user has a parallel setup and doesn't want to upload any videos, but still wants failed videos to be saved on some machine somewhere. Uploading and recording are two very different things, and I'm now (at the very least) the second person to not understand that the Having two switches makes it super clear: Now, I can save only passing or failing videos, both, or none, and Cypress will upload whatever videos happen to be there. Edit: I've thrown in "pass" as an option here too, since once a working "only failed tests" switch is working, negating this value based on the config should be trivial, and gives users added functionality |
|
I agree that it leaves some functionality to be desired, but the current behaviour seems to do what you and I are asking for. |
This comment has been minimized.
This comment has been minimized.
|
@GrayedFox I like your proposal for As it stands right now setting |
This comment has been minimized.
This comment has been minimized.
jennifer-shehane
added
stage: proposal 💡
This comment has been minimized.
This comment has been minimized.
|
@brian-mann in response to your comment here - where you ask why this feature is necessary, I think this thread highlights valid use cases, perhaps? Also I would argue that rather than asking the contributors and users of Cypress why they want to be able to control how they use Cypress, I would ask (aside from having the video recording have default values that you can build other features on top of it): is the video recording feature really so tightly coupled with dashboard functionality that it cannot be disabled without breaking the dashboard? I'd need to take a better look at the code but that sounds like we may be breaking the single responsibility principle here. Ideally, the dashboard functions as a graphical interface for whatever pluggable features the Cypress user thinks they need... even if you know better and think that real life use cases would necessitate that all people should be using feature X or Y, that doesn't seem to me to be a reason to have such tightly coupled code. That, and using something like feature flags for the dash also means you can rapidly disable any feature which (god forbid!) ships with a serious bug, while working on a hotfix, without disabling other parts of the system that work (just food for thought!). Thanks again for steering this wonderful product through the murky waters of open source shenanigans - I'm sure comments like these ones bring you immense happiness and joy 🙇 |
|
I'm completely on your page @GrayedFox, very thankful for the cypress project itself and would not be testing my frontends like this if it wouldn't be there... @brian-mann @jennifer-shehane let's focus on a solution here, the other ticket is closed and probably won't get much attention for others following along currently. For reference, here are the last two comments on my deprecated issue, cited:
|
Add a note about how the `videoUploadOnPasses` option can be used to avoid processing and uploading videos in the case that all tests pass. Partially addresses cypress-io/cypress#2217, cypress-io/cypress#2522
* Update screenshots-and-videos.md Add a note about how the `videoUploadOnPasses` option can be used to avoid processing and uploading videos in the case that all tests pass. Partially addresses cypress-io/cypress#2217, cypress-io/cypress#2522 * Update screenshots-and-videos.md Remove autoformatter-caused changes * Update to clarify that videos process per spec file run - '+' to 'and' Co-authored-by: Jennifer Shehane <shehane.jennifer@gmail.com>
This comment has been minimized.
This comment has been minimized.
|
This feature would be very appreciated. |
jennifer-shehane
changed the title
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Well @kjellski, my To reiterate, we want the ability to record videos only for failed tests. A possible implementation was described at #2522 (comment): I think it's clear by now that the community wants this feature. The specification was already laid out by @GrayedFox. Is there anything blocking the implementation of this feature? |
|
@dialex aaahhhh, you only want the ones for failed! Sorry - I was not expecting this restriction - just as a workaround, you could just delete the successfull ones with a little script before ending your 'build' for now... But yes, this seems to be not existing yet AFAIK! |
This comment has been minimized.
This comment has been minimized.
|
In my company, I'm currently working to help other teams to move on to Cypress. Our setup runs Cypress on Jenkins hosts, and are orchestrated with Cypress Dashboard. The videos are automatically saved in Jenkins. There is no way to get around this, other than completely disabling video in the configuration. One of the current issues hindering wider adoption is how much space these videos take up per Jenkins Job, so disabling compression isn't an option for us. While deleting the videos after the fact would help with this space issue, the videos will still be saved in the first instance. I'd like to increase the parallelisation on our cypress tests, but we are currently running into i/o bottlenecks on Jenkins. Saving then deleting in quick succession will not help with this Deleting a saved video in an |
aside from disabling videos entirely, how would you expect to solve your problem? The only thing I can think of that would reduce space, is to ensure that you can hook into the after spec as each spec is run and not only right at the end of all tests, so if all the tests pass the most space that would be used is the space of the largest single video file. @jennifer-shehane |
Given that there is a saving/compression step after each spec, I am assuming that the videos are in memory until they are saved to disk. Is that not the case? Edit- thanks also for the helpful suggestion :) |
|
@liambutler if you really want to only record videos for failed tests, the solution would be to run all failed tests twice (and hope you get the same failure again, which in my experience is likely, but this will cause headaches for flaky tests/envs). You could set up a default config where videos are disabled entirely, and then you identify each failed test (easy enough to do after the fact or during runtime). Save the name of each failed test and construct a comma delimited string and then re-run only those failed tests with videos enabled, so with a @jennifer-shehane out of interest when exactly does that after hook trigger?
If it's either at step 1 or 2 I believe this proposal solves my (and most others) concerns. If it's at step 3, then it only helps solve the space issue, since it allows for a delete operation after each passing spec instead of at the end of the test suite. |
|
Still hoping that the solution I had in mind is possible, but thanks for all the tips! I love how helpful the Cypress community is toward each other |
|
Related PRs to this feature:
|
|
In Cypress 6.2.0, you can now listen to This means that you can delete the video after a spec has run for any reason - including if all of the tests passed. This alone will not skip the processing/compression of the video. If you want to skip the processing time for the video, you can set the To skip processing time for all videos:
{
"videoCompression": false
}To delete a video if all tests pass:
{
"experimentalRunEvents": true
}`cypress/plugins/index.js // 'npm install -D del' - https://www.npmjs.com/package/del
const del = require('del')
module.exports = (on, config) => {
on('after:spec', (spec, results) => {
if (results.stats.failures === 0 && results.video) {
// `del()` returns a promise, so it's important to return it to ensure
// deleting the video is finished before moving on
return del(results.video)
}
})
}Let us know if you have any feedback on this experimental feature. |
|
Unless people have specific concerns with how this issue is resolved using the experimental feature, I believe the core concerns have been addressed. Thank you to @jennifer-shehane for your continued patience and persistence in getting this resolved and navigating both the external and internal communications channels, and to all those who used this thread constructively. As with many a good thing, they can and do take time. |
|
tried to use the approved solution but getting "fast-glob\out\readers\sync.js: Unexpected token" on "cypress": "^6.2.1". Anything I miss?
|
|
@nichtmeinweg I did mispell |
Thanks a lot for the correction, my error is most probably related to the needed upgrade of cypress I had to do for following the workaround. Which version of "del" are you using? it is not related to the flag: "experimentalRunEvents", or the EVENT function, its actually this line that causes the error: const del = require('del');Dependencies: |
Doesn't this disable compression for all videos then? I'd still like to compress videos for failed tests. |
It does. You can set |
|
@josefsmrz That's only for Cypress videos uploaded to the Dashboard, though. It seems like deleting the video via the Edit: good catch, looks like it does disable processing: cypress/packages/server/lib/modes/run.js Line 826 in 0156a3e |
@caseyjhol I guess it is intended mainly for use with the Dashboard, but as a side effect, the setting actually does what I've described even when not using the Dashboard. It has been discussed in the very beginning of this thread |
@jennifer-shehane This seems to work very well for us. We were running out of space on our CI server and large videos were the main culprit. In addition to freeing up lots of space, we save valuable time by not processing the video too. The rare fix that saves both time and space! 😄 |
|
Is anyone else noticing (after implementing the suggestion here) that now there are warnings about missing video files? I'm seeing this for all my tests now: Is there any way to silence these warnings? |
yeah also got them |
|
I get the same error about missing videos. Perhaps one possibility to avoid this is to collect file paths and delete videos in import {rm} from 'fs/promises'
const pluginConfig = (on, config) => {
const filesToDelete = []
on('after:spec', (spec, results) => {
if (results.stats.failures === 0 && results.video) {
filesToDelete.push(results.video)
}
})
on('after:run', async () => {
console.log(
'Deleting %d videos from successful specs',
filesToDelete.length
)
await Promise.all(filesToDelete.map((videoFile) => rm(videoFile)))
})
}
export default pluginConfigNot that this will delete videos only when ran through Also, this particular snippet won't work with Node 12 currently bundled by Cypress due to lack of |
|
Yes, that works without the error message, I just changed the "after:run" hook with: NodeJS 14.5.1 |
|
I've added this to my current setup, minus the experimental features part and the video compression which I believe are not needed for this to work fully now. The process works locally for me but when I pushed the new plugin/index.js to my repo and ran a test from Jenkins I am getting an error about the plugin/index.js file not being found (see below). I am guessing this is more than likely a config error on Jenkins rather than something related to the improvement suggested here but I am hoping someone else may have experienced this issue and may have a solution? The plugins file is missing or invalid. Your Or you might have renamed the extension of your Please fix this, or set Error: Cannot find module 'del' |
|
@kaiyoma We've updated the 'warning' message for when the video is missing for processing. Since this is likely expected behavior that someone coded in (to delete the video), we replaced the messaging with a single line so that you are still informed that the video processing is being skipped. #15828 |
|
The This issue will be closed to further comment as the exact issue feature here is delivered. See #2522 (comment) for an example. If you're experiencing a bug related to the issue above in Cypress, please open a new issue with a fully reproducible example that we can run. There may be a specific edge case with the issue that we need more detail to fix. |
|
Uploaded this comment to show solution for controlling which videos to delete in newer versions: https://docs.cypress.io/guides/guides/screenshots-and-videos#Control-which-videos-to-keep-and-upload-to-Cypress-Cloud |
and others
Current behavior:
Currently, it's only possible to switch the video feature on or off in a binary way.
Desired behavior:
To be able to record and process videos only when a test fails. If there's no way to do this (due to the recording feature not knowing whether a test will fail or not when it begins to record) - the processing part could at least be tweaked so that only failed tests have their videos processed and saved, whereas passing tests are technically recorded, but never processed or saved to disk if the test passes.
Versions
Cypress 3.1.0
Chrome 69
Electron 59
The text was updated successfully, but these errors were encountered: