Skip to content

Removed react-16.8 from allowed_failures in travis#2122

Open
AlokTakshak wants to merge 1 commit intoenzymejs:masterfrom
AlokTakshak:closes2113
Open

Removed react-16.8 from allowed_failures in travis#2122
AlokTakshak wants to merge 1 commit intoenzymejs:masterfrom
AlokTakshak:closes2113

Conversation

@AlokTakshak
Copy link
Copy Markdown

@AlokTakshak AlokTakshak commented May 11, 2019

Closes #2113.

@AlokTakshak AlokTakshak changed the title Removed react-16.8, react-16.8.5 from allowed_failures in travis Removed react-16.8 from allowed_failures in travis May 16, 2019
@AlokTakshak
Copy link
Copy Markdown
Author

@ljharb
I have removed react 16.8 from allowed_failures

.travis.yml Outdated
env: REACT=16.8.3
- node_js: "lts/*"
env: REACT=16.8 RENDERER=16.7
env: REACT=16.8 RENDERER=16.8
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is breaking the purpose of the test, which is to test react 16.8 with the 16.7 renderer.

Suggested change
env: REACT=16.8 RENDERER=16.8
env: REACT=16.8 RENDERER=16.7

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

But we need corresponding version of react and react-dom if there is version mismatch - hooks test will fail as writtern on react website one of reason for invalid hooks is
Mismatching Versions of React and React DOM

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

react and react-dom will both be 16.8 with this. RENDERER is about react-test-renderer, which doesn't necessarily have to match.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

ok , then i can use render, fireEvent, getByTestId for updating the failing test cases?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The work that needs to be done to close the linked issue is to determine if the existing tests are valid - if so, the actual code needs to be fixed; or, if they're not valid, change them so they are valid.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@ljharb I was going through a lot of articles the react-test-renderer before 16.8.5 have problem with shallow rendering for hooks , as they won't re-render after setting State , and we are using same the same in Adaptors so shallow won't work for version below 16.8.5,
now what we should do, modify the adaptor?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes, ideally we'd modify the react 16 adapter to have fallback behavior for an older renderer version.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@AlokTakshak any interest in pursuing this fix?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I want to but i don't have any clue of how to fix it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My thinking is that the react 16 adapter would add some way (only for react-test-renderer prior to 16.8.5) to catch useState calls and force a rerender manually.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 21, 2020

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.12%. Comparing base (0e39e1f) to head (3654397).
⚠️ Report is 116 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2122   +/-   ##
=======================================
  Coverage   96.12%   96.12%           
=======================================
  Files          49       49           
  Lines        4004     4004           
  Branches     1123     1123           
=======================================
  Hits         3849     3849           
  Misses        155      155           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ljharb ljharb force-pushed the master branch 4 times, most recently from 2227326 to 0d5ead7 Compare December 21, 2020 07:46
@ljharb ljharb force-pushed the master branch 3 times, most recently from 43eb75e to 39e6b1f Compare November 3, 2022 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

hooks: fix failing tests with mismatched renderer

2 participants