breaking: remove the shouldUploadVideoOnPass configuration

[AtofStryker]

• Closes Remove videoUploadOnPasses as a configuration option #26899

Additional details

see #26899 for details/rationale

Steps to test

How has the user experience changed?

PR Tasks

• Have tests been added/updated?
• Has a PR for user-facing changes been opened in cypress-documentation? BREAKING: remove the videoUploadOnPasses configuration option cypress-documentation#5306
• Have API changes been updated in the type definitions?

[AtofStryker]

snapshot is different because we are now deleting the video entirely, which is slightly different than videoUploadOnPasses, which keeps the video but skips compression and does NOT upload it to the cloud.

[cypress]

11 flaky tests on run #47963 ↗︎

0

10028

986

0

11

Details:

run ci
Project: cypress	Commit: abef79b169
Status: Passed	Duration: 14:16 💡
Started: Jun 21, 2023 3:10 PM	Ended: Jun 21, 2023 3:24 PM

  specs_list_latest_runs.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
App/Cloud Integration - Latest runs and Average duration > when no runs are recorded > shows placeholders for all visible specs		Output Screenshots

  cypress-origin-communicator.cy.ts • 1 flaky test • app-e2e

View Output Video

Test		Artifacts
Cypress In Cypress Origin Communicator > cy.origin passivity with app interactions > passes upon test reload mid test execution		Output Screenshots

  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-electron

View Output Video

Test		Artifacts
... > correctly returns currentRetry		Output
... > correctly returns currentRetry		Output
... > correctly returns currentRetry		Output

  commands/net_stubbing.cy.ts • 3 flaky tests • 5x-driver-webkit

View Output Video

Test		Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse		Output
... > with `times` > only uses each handler N times		Output
... > stops waiting when an xhr request is canceled		Output

  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-webkit

View Output Video

Test		Artifacts
... > correctly returns currentRetry		Output
... > correctly returns currentRetry		Output
... > correctly returns currentRetry		Output

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[brian-mann]

@AtofStryker I know that this PR is already merged - however even though this is a breaking change, you can't just remove this config and silently swallow config that has that property present.

In fact if you look at other breaking config change patterns - whenever we remove/rename config, we always leave a "check" in there that if the removed property still exists in the config we throw a nice error saying that this was removed as of vX.X.X.

If you look at other proxies for this pattern you'll find them in the code and in the error messages. Basically what we don't want to have happen is that this property is removed, but you keep the old value around and yet it no longer "works" or applies, and there's nothing prompting you that something is wrong. By throwing an error the user gets a very loud message indicating this was removed. The error message can also naturally guide them to know what to do in response to it being removed.

[brian-mann]

Here's examples of that pattern. If you look at the error messages you can see the pattern of "guiding the users to recover from the failure"

[AtofStryker]

@brian-mann apologies for missing that. I created #27158 which should warn the user in the terminal if the videoUploadOnPasses is present so the user can remove it. I didn't elect to error since the config option is somewhat passive, but if you feel different let me know.

[cypress-bot]

Released in 13.0.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v13.0.0, please open a new issue.