-
Notifications
You must be signed in to change notification settings - Fork 217
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
test: Remove enzyme #1418
test: Remove enzyme #1418
Conversation
declare global { | ||
interface Window { | ||
resizeBy: (x: number, y: number) => void; | ||
} | ||
} | ||
const map: {[key: string]: any} = {}; | ||
window.addEventListener = jest.fn((event, callback) => { | ||
map[event] = callback; | ||
}); | ||
window.removeEventListener = jest.fn((event, callback) => { | ||
if (map[event] && map[event] === callback) { | ||
delete map[event]; | ||
} | ||
}); | ||
window.dispatchEvent = (event: Event) => { | ||
if (map[event.type]) { | ||
map[event.type](); | ||
} | ||
|
||
// TODO: not totally accurate | ||
return false; | ||
}; | ||
|
||
window.resizeBy = (x: number, y: number) => { | ||
// @ts-ignore | ||
window.innerWidth = x; | ||
// @ts-ignore | ||
window.innerHeight = y; | ||
window.dispatchEvent(new Event('resize')); | ||
}; | ||
|
||
// @ts-ignore | ||
window.requestAnimationFrame = cbFn => cbFn(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Luckily none of this is necessary anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha BYE! ✌🏻
Test summaryRun details
View run in Cypress Dashboard ➡️ 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 |
…ove-enzyme # Conflicts: # modules/react/icon/spec/Svg.spec.tsx # yarn.lock
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for removing Enzyme!
declare global { | ||
interface Window { | ||
resizeBy: (x: number, y: number) => void; | ||
} | ||
} | ||
const map: {[key: string]: any} = {}; | ||
window.addEventListener = jest.fn((event, callback) => { | ||
map[event] = callback; | ||
}); | ||
window.removeEventListener = jest.fn((event, callback) => { | ||
if (map[event] && map[event] === callback) { | ||
delete map[event]; | ||
} | ||
}); | ||
window.dispatchEvent = (event: Event) => { | ||
if (map[event.type]) { | ||
map[event.type](); | ||
} | ||
|
||
// TODO: not totally accurate | ||
return false; | ||
}; | ||
|
||
window.resizeBy = (x: number, y: number) => { | ||
// @ts-ignore | ||
window.innerWidth = x; | ||
// @ts-ignore | ||
window.innerHeight = y; | ||
window.dispatchEvent(new Event('resize')); | ||
}; | ||
|
||
// @ts-ignore | ||
window.requestAnimationFrame = cbFn => cbFn(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haha BYE! ✌🏻
@@ -274,7 +274,7 @@ class MenuItem extends React.Component<MenuItemProps> { | |||
id={id} | |||
role={role} | |||
onClick={this.handleClick} | |||
aria-disabled={!!isDisabled} | |||
aria-disabled={isDisabled ? true : undefined} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We didn't catch this before, but this was actually a bug.
@@ -51,7 +53,7 @@ jobs: | |||
- uses: actions/cache/@v2 | |||
with: | |||
path: .cache/cypress | |||
key: ${{ runner.os }}-cypress-cache-version-${{ steps.cypress-version.outputs.version }} | |||
key: ${{ runner.os }}-cypress-cache-version-${{ needs.install.outputs.cypress-version }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When everything was under a single job, the steps context could be used to get outputs of a specific step. When we parallelized the workflow, we had to break out the workflow into multiple jobs with dependencies to make sure everything ran in the proper order. Github doesn't allow a step to access the output of another job. I never updated the calls for the cypress-version
string in the jobs. Instead, you have to add outputs
to a job. From there, a different job can access those job outputs if the needs is added (meaning depending on the job). Then you can access those outputs via needs.{job_id}.outputs.{output_name}
. So I had to add job outputs and update downstream jobs to access this output.
So instead of Github failing with a "could not find" something error message, it simply returns blank. Somehow this now invalid cache key returns Cypress 5.6.0 instead of 9.2.0. My guess is the invalid cache key will return Cypress 5.6.0 or 9.2.0 on whether the support
or master
job was run last that warmed the cache. I happened to find this because I ran a PR off master soon after a job on support primed the cache with the 5.6.0 version. Originally I found it because I changed Cypress from 9.2.0 to 9.2.1 and thus the binary missing. I thought it was just my PR, but that was a coincidence
Summary
Enzyme is not being supported long term. The React 17 adapter is unofficial and there may not be an adapter for future versions of React.
Additional Notes
I was able to remove extra setup code required by Enzyme, but not by React Testing Library