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

chore: pass file preprocesor override to protocol as debug data #31171

Merged

Conversation

ryanthemanuel
Copy link
Collaborator

@ryanthemanuel ryanthemanuel commented Feb 26, 2025

Additional details

In order to improve troubleshooting source map issues as a part of test generation, we are going to start capturing any file:preprocessor overrides. This will hopefully give us insight into whether or not sourcemaps have not been properly enabled. This PR captures the function string and passes it from the child process to main Cypress where it's stored as a debug property on ProjectConfigLifecycleManager. There it can be passed to protocol where it will be stored as a debug event.

Steps to test

How has the user experience changed?

PR Tasks

@ryanthemanuel ryanthemanuel changed the base branch from develop to ryanm/chore/studio-types February 26, 2025 03:33
@ryanthemanuel ryanthemanuel changed the title chore: set up sharing of react via module federation in studio chore: pass file preprocesor override to protocol as debug data Feb 26, 2025
Copy link

cypress bot commented Feb 26, 2025

cypress    Run #60702

Run Properties:  status check passed Passed #60702  •  git commit fe02ec18db: merge develop
Project cypress
Branch Review ryanm/chore/pass-file-preprocessor-override-to-protocol
Run status status check passed Passed #60702
Run duration 17m 59s
Commit git commit fe02ec18db: merge develop
Committer Ryan Manuel
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 11
Tests that did not run due to a developer annotating a test with .skip  Pending 1232
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 32101
View all changes introduced in this branch ↗︎
UI Coverage  46.15%
  Untested elements 184  
  Tested elements 162  
Accessibility  92.55%
  Failed rules  3 critical   8 serious   2 moderate   2 minor
  Failed elements 877  

Comment on lines 131 to 132
registerChildEvent('file:preprocessor', this._getDefaultPreprocessor(initialConfig), true)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of updating registerChildEvent which seems odd since we would need to update all of our registerChildEvent calls to pass true, can we do something like:

Suggested change
registerChildEvent('file:preprocessor', this._getDefaultPreprocessor(initialConfig), true)
}
registerChildEvent('file:preprocessor', this._getDefaultPreprocessor(initialConfig), true)
} else {
const handler = this.registeredEventsById[this.registeredEventsByName['file:preprocessor']].handler
this.ipc.send('file:preprocessor:overridden', { handlerString: handler.toString() })
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like it. Updated: 1a261e2 (#31171)

@@ -238,6 +244,12 @@ export class ProjectConfigIpc extends EventEmitter {
reject(err)
})

this.once('file:preprocessor:overridden', ({ handlerString }) => {
this.onDebugData({
filePreprocessorHandlerString: handlerString,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
filePreprocessorHandlerString: handlerString,
filePreprocessorHandlerText: handlerFunctionText,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call. Updated: 1a261e2 (#31171)

Copy link
Contributor

@mschile mschile left a comment

Choose a reason for hiding this comment

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

Looks good overall! Added a couple suggestions.

@@ -238,6 +244,12 @@ export class ProjectConfigIpc extends EventEmitter {
reject(err)
})

this.once('file:preprocessor:overridden', ({ handlerString }) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this.once('file:preprocessor:overridden', ({ handlerString }) => {
this.once('file:preprocessor:overridden', ({ handlerText }) => {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Base automatically changed from ryanm/chore/studio-types to develop February 28, 2025 22:30
@ryanthemanuel ryanthemanuel merged commit bd4774f into develop Mar 3, 2025
89 of 91 checks passed
@ryanthemanuel ryanthemanuel deleted the ryanm/chore/pass-file-preprocessor-override-to-protocol branch March 3, 2025 18:52
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Mar 12, 2025

Released in 14.2.0.

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

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Mar 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Protocol: Capture project metadata about whether sourcemaps are inlined
3 participants