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

fix: test config overrides leak for .only execution #18961

Merged
merged 10 commits into from Nov 22, 2021

Conversation

emilyrohrbough
Copy link
Member

Recent change to add configuration validation required updates to include the invocation details to throw an invalidation configuration error with the correct line number of where the configuration was passed in for suite/test configuration. These changed did not account for single suite/test execution which was normalized more than once.

How has the user experience changed?

The configuration appears to have been applying correctly, so should be minimal to no impact on consumers.

PR Tasks

  • Have tests been added/updated?
  • [] Has the original issue or this PR been tagged with a release in ZenHub?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 17, 2021

Thanks for taking the time to open a PR!

@emilyrohrbough emilyrohrbough changed the title fix[driver]: test config overrides leak for .only excution fix: test config overrides leak for .only execution Nov 17, 2021
@cypress
Copy link

cypress bot commented Nov 17, 2021



Test summary

18701 0 202 0Flakiness 3


Run details

Project cypress
Status Passed
Commit c87568a
Started Nov 22, 2021 2:54 PM
Ended Nov 22, 2021 3:05 PM
Duration 11:23 💡
OS Linux Debian - 10.10
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

commands/net_stubbing_spec.ts Flakiness
1 network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"
e2e/redirects_spec.js Flakiness
1 redirection > meta > binds to the new page after a timeout
cypress/proxy-logging-spec.ts Flakiness
1 Proxy Logging > request logging > xhr log has response body/status code

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@emilyrohrbough emilyrohrbough marked this pull request as ready for review November 17, 2021 15:39
@emilyrohrbough emilyrohrbough requested a review from a team as a code owner November 17, 2021 15:39
@emilyrohrbough emilyrohrbough requested review from jennifer-shehane and removed request for a team November 17, 2021 15:39
Comment on lines 36 to 52
systemTests.it(`correctly applies overrides when valid configuration for describe.only`, {
spec: 'testConfigOverrides-describe-only-valid.js',
snapshot: true,
expectedExitCode: 0,
config: {
video: false,
},
})

systemTests.it(`correctly applies overrides when valid configuration for it.only`, {
spec: 'testConfigOverrides-it-only-valid.js',
snapshot: true,
expectedExitCode: 0,
config: {
video: false,
},
})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these don't need to be system tests, they can be regular driver Cypress tests instead, right? Since we run one test file at a time in cypress run, it should work the same. packages/driver/cypress/integration/e2e would be a good place.

Avoid adding system tests if possible since they're a lot heavier, resource-wise. And since we're just running the test and the snapshot doesn't have any additional info in it, it should be fine to make these regular Cypress tests.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm good point. Updated: 96e4e80 (#18961)

@jennifer-shehane jennifer-shehane removed their request for review November 18, 2021 14:58
@emilyrohrbough emilyrohrbough merged commit b7002bf into develop Nov 22, 2021
@emilyrohrbough emilyrohrbough deleted the fix-test-config-only branch November 22, 2021 15:10
tgriesser added a commit that referenced this pull request Nov 28, 2021
* develop:
  test: node_modules installs for system-tests, other improvements (#18574)
  chore(deps): update dependency semantic-release to v17.2.3 [security] (#19022)
  chore: remove flaky ci jobs for main builds (#19071)
  chore(contributing): clarify PULL_REQUEST_TEMPLATE (#19068)
  fix: the shadow root container element is ignored when clicking an element in it. (#18908)
  'Fix' flaky redirect test (#19042)
  release 9.1.0 [skip ci]
  fix: Allow 'this' to be used in overridden commands (#18899)
  fix(react): link to rerender example (#19020)
  chore(deps): update dependency aws-sdk to v2.814.0 [security] (#18948)
  fix: test config overrides leak for .only execution (#18961)
  feat: Set CYPRESS=true as env var in child processes where Cypress runs user code in Node (#18981)
  fix: Restore broken gif (#18987)
  chore: release @cypress/vite-dev-server-v2.2.1
tgriesser added a commit that referenced this pull request Nov 29, 2021
* 10.0-release:
  feat(graphql): ability to update/query for appData (#19082)
  fix system test
  fix failing tests due to merge
  resolve conflicts
  test: node_modules installs for system-tests, other improvements (#18574)
  update yarn.lock
  chore(deps): update dependency semantic-release to v17.2.3 [security] (#19022)
  chore: remove flaky ci jobs for main builds (#19071)
  chore(contributing): clarify PULL_REQUEST_TEMPLATE (#19068)
  fix: the shadow root container element is ignored when clicking an element in it. (#18908)
  'Fix' flaky redirect test (#19042)
  release 9.1.0 [skip ci]
  fix: Allow 'this' to be used in overridden commands (#18899)
  fix(react): link to rerender example (#19020)
  chore(deps): update dependency aws-sdk to v2.814.0 [security] (#18948)
  fix: test config overrides leak for .only execution (#18961)
  feat: Set CYPRESS=true as env var in child processes where Cypress runs user code in Node (#18981)
  fix: Restore broken gif (#18987)
  chore: release @cypress/vite-dev-server-v2.2.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test Configuration data leaking into configuration for describe.only & it.only blocks
3 participants