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: runnable header open in IDE and filename #19630

Merged
merged 10 commits into from
Jan 10, 2022

Conversation

ZachJW34
Copy link
Contributor

User facing changelog

Add filename to reporter runnables header and ensure update of filename and open in IDE path when selected spec changes

Additional details

The reporter runnables header didn't show the filename and and the open in IDE path became stale when switching specs (it would only open the first spec that was selected). We now pass in the runnerStore so that React can update when the runnerStore updates.

How has the user experience changed?

Screen.Recording.2022-01-10.at.9.52.28.AM.mov

PR Tasks

  • Have tests been added/updated?
  • [na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?
  • [na] Have new configuration options been added to the cypress.schema.json?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jan 10, 2022

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Jan 10, 2022



Test summary

18560 0 219 0Flakiness 2


Run details

Project cypress
Status Passed
Commit 08ad5c5
Started Jan 10, 2022 10:15 PM
Ended Jan 10, 2022 10:26 PM
Duration 11:39 💡
OS Linux Debian - 10.10
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

cypress/integration/commands/net_stubbing_spec.ts Flakiness
1 network stubbing > waiting and aliasing > can timeout incrementally waiting on requests
2 network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"

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

Copy link
Contributor

@elevatebart elevatebart left a comment

Choose a reason for hiding this comment

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

Some comments, nothing big.
Is it expected that so many test suites fail?

@@ -122,7 +122,7 @@ class Reporter extends Component<SingleReporterProps> {

shortcuts.start()
EQ.init()
this.props.runnablesStore.setRunningSpec(spec.relative)
this.props.runnablesStore.setRunningSpec(this.props.runnerStore.spec.relative)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could runnerStore be destructured like the rest of the props for consistency?

if (window.Cypress) {
window.state = appState
window.render = (props) => {
if (_window.Cypress) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment about why the _ ?
Is it to avoid TypeScript validation?

If it is, use expect error more than any if you can.
You could also augment the window object if you need to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was before I added lib: ["DOM"] to the tsconfig.json, I can remove the these changes

import TextIcon from '-!react-svg-loader!@packages/frontend-shared/src/assets/icons/document-text_x16.svg'
import FileNameOpener from '../lib/file-name-opener'
Copy link
Contributor

Choose a reason for hiding this comment

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

This move threw me off for a bit ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I blame it on VS code!

@ZachJW34
Copy link
Contributor Author

@elevatebart Everything should be passing, is this with regards to the CI tests or the changes made to the reporter tests?

@elevatebart
Copy link
Contributor

@ZachJW34 I was referring to CI

@ZachJW34
Copy link
Contributor Author

@elevatebart All passing now!

Copy link
Contributor

@elevatebart elevatebart left a comment

Choose a reason for hiding this comment

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

GOGOOOGOGO

@ZachJW34 ZachJW34 merged commit 114ce10 into 10.0-release Jan 10, 2022
@ZachJW34 ZachJW34 deleted the fix-reporter-runnable-header branch January 10, 2022 23:17
tgriesser added a commit that referenced this pull request Jan 11, 2022
* 10.0-release:
  chore: change integration->e2e throughout code base (#19345)
  fix(unify): correct location of snapshot pinning UI in runner (#19640)
  fix: runnable header open in IDE and filename (#19630)
  feat(unify): allow prompts to auto-open after set time (#19571)
  test(unify): Additional navigation tests for log in workflows (#19574)
  fix(unify): minor UI bugs in the reporter (#19445)
  fix: surface openSSL unhandled rejection errors that occur within plugin code (#19598)
  feat: add spec watcher to data-context (#19583)
tgriesser added a commit that referenced this pull request Jan 13, 2022
* 10.0-release: (28 commits)
  fix: flake in shelljs scripts during postinstall (#19678)
  fix: various lifecycle issues, followup to #19572 (#19683)
  feat: error screens in app when cloud runs are failing (#18980)
  fix: open browser at correct time during lifecycle (#19572)
  chore: remove unused types (#19621)
  fix: fileparts processing and cleanup (#19660)
  chore: change integration->e2e throughout code base (#19345)
  fix(unify): correct location of snapshot pinning UI in runner (#19640)
  fix: runnable header open in IDE and filename (#19630)
  feat(unify): allow prompts to auto-open after set time (#19571)
  test(unify): Additional navigation tests for log in workflows (#19574)
  fix(unify): minor UI bugs in the reporter (#19445)
  fix: surface openSSL unhandled rejection errors that occur within plugin code (#19598)
  feat: add spec watcher to data-context (#19583)
  chore: remove unused components (#19618)
  fix: dev-server types (#19576)
  test: ct audit updates (#19464)
  chore: fix temporarily skipped specs on 10.0-release (#19611)
  fix: remove gremlins characters from cypress types
  freeze ipc
  ...
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.

2 participants