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.

Sign up for GitHub

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

Jump to bottom

chore: Fix tests #1288

Merged
merged 4 commits into from
Mar 19, 2024
Merged

chore: Fix tests #1288

merged 4 commits into from
Mar 19, 2024

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Mar 19, 2024 •
edited
Loading

Fixing

[test] FAIL src/__tests__/render.js (9.164 s)
[test]   ● render API › legacyRoot uses legacy ReactDOM.render
[test] 
[test]     TypeError: jestDiff is not a function
[test] 
[test]       161 |           errorMessage =
[test]       162 |             'Unexpected warning recorded: ' +
[test]     > 163 |             jestDiff(expectedMessages[0], normalizedMessage)
[test]           |             ^
[test]       164 |         } else {
[test]       165 |           errorMessage =
[test]       166 |             'Unexpected warning recorded: ' +
[test] 
[test]       at console.jestDiff (tests/toWarnDev.js:163:13)
[test]       at printWarning (node_modules/react-dom/cjs/react-dom.development.js:105:30)
[test]       at error (node_modules/react-dom/cjs/react-dom.development.js:79:7)
[test]       at Object.render (node_modules/react-dom/cjs/react-dom.development.js:37824:5)
[test]       at Object.render (src/pure.js:133:16)
[test]       at render (src/pure.js:152:12)
[test]       at callback (src/act-compat.js:42:24)
[test]       at act (node_modules/react/cjs/react.development.js:2719:16)
[test]       at actImplementation (src/act-compat.js:[41](https://github.com/testing-library/react-testing-library/actions/runs/8349938959/job/22855248000?pr=1287#step:7:42):25)
[test]       at renderRoot (src/pure.js:1[45](https://github.com/testing-library/react-testing-library/actions/runs/8349938959/job/22855248000?pr=1287#step:7:46):6)
[test]       at renderRoot (src/pure.js:241:10)
[test]       at src/__tests__/render.js:218:13
[test]       at Object.callback (tests/toWarnDev.js:185:9)
[test]       at __EXTERNAL_MATCHER_TRAP__ (node_modules/expect/build/index.js:325:30)
[test]       at Object.throwingMatcher [as toErrorDev] (node_modules/expect/build/index.js:326:15)
[test]       at Object.toErrorDev (src/__tests__/render.js:219:8)

-- https://github.com/testing-library/react-testing-library/actions/runs/8349938959/job/22855248000?pr=1287#step:7:43

Also adjusts to changed warnings in facebook/react#28477

@codesandbox-ci CodeSandbox CI
Copy link

codesandbox-ci bot commented Mar 19, 2024 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c4b23e4:

Sandbox Source
react-testing-library-examples Configuration

eps1lon
eps1lon commented Mar 19, 2024
View reviewed changes
tests/toWarnDev.js
@@ -29,9 +29,12 @@ SOFTWARE.
/* eslint-disable func-names */
/* eslint-disable complexity */
const util = require('util')
const jestDiff = require('jest-diff').default
const jestDiff = require('jest-diff').diff
Copy link
Member Author

Choose a reason for hiding this comment

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

No idea why this ever worked. It didn't change in jest-diff between 29.4.1 and 29.7.0 so it must have been some other thing that made it work incidentally.

@eps1lon eps1lon force-pushed the fix-tests branch 2 times, most recently from 58345d3 to 013c14c Compare March 19, 2024 21:42
@eps1lon eps1lon marked this pull request as ready for review March 19, 2024 22:19
@eps1lon
Copy link
Member Author

eps1lon commented Mar 19, 2024

Will fix codecov in a follow-up. Manual inspection shows 100%: https://app.codecov.io/github/testing-library/react-testing-library/commit/c4b23e4fd087fd27b95b2051de5fe156f7a07523

@eps1lon eps1lon merged commit 7e42f4e into testing-library:main Mar 19, 2024
11 checks passed
@eps1lon eps1lon deleted the fix-tests branch March 19, 2024 22:24
MatanBobi
MatanBobi reviewed Mar 20, 2024
View reviewed changes
src/__tests__/render.js
? [
"Warning: ReactDOM.render is no longer supported in React 18. Use createRoot instead. Until you switch to the new API, your app will behave as if it's running React 17. Learn more: https://react.dev/link/switch-to-createroot",
]
: [
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need this? the reactjs.org link has a redirect to the react.dev one. Is this some sort of preparation for future stuff?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's what React is logging

MatanBobi
MatanBobi reviewed Mar 20, 2024
View reviewed changes
src/__tests__/render.js
@@ -3,6 +3,13 @@ import ReactDOM from 'react-dom'
import ReactDOMServer from 'react-dom/server'
import {fireEvent, render, screen, configure} from '../'

// Needs to be changed to 19.0.0 once alpha started.
const isReactExperimental = React.version.startsWith('18.3.0-experimental')
const isReactCanary = React.version.startsWith('18.3.0')
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we check for React.version.startsWith('18.3.0-canary') here?

Copy link
Member Author

Choose a reason for hiding this comment

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

yep

@MatanBobi MatanBobi mentioned this pull request Mar 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MatanBobi MatanBobi MatanBobi left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eps1lon @MatanBobi