Skip to content

fix: toHaveTextContent supports interpolated variables #59

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

Conversation

jeremyshearer
Copy link
Contributor

@jeremyshearer jeremyshearer commented Jun 12, 2021

What:

Fix a bug where text content that gets split into an array does not get evaluated by the toHaveTextContent matcher.

Why:

There's a test case in the PR, but this test case currently fails in master:

    const variable = 'variable';
    const { container } = render(<Text>With a {variable}</Text>);

    expect(container).toHaveTextContent('With a variable');

    // Expected element to have text content:
         // With a variable
    // Received:

How:
The bug was in treating all array elements as react nodes that had a path of ['props', 'children'] that could be traversed to get the text content. The fix is to pass the element directly into the recursive getText call and let the conditional logic determine how to extract the text.

Checklist:

  • Documentation added to the
    docs N/A
  • Typescript definitions updated N/A
  • Tests
  • Ready to be merged

@codecov
Copy link

codecov bot commented Jun 12, 2021

Codecov Report

Merging #59 (2767b49) into master (3cfd279) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 2767b49 differs from pull request most recent head 4f2d3d3. Consider uploading reports for the commit 4f2d3d3 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##            master       #59   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines           96       147   +51     
  Branches        28        48   +20     
=========================================
+ Hits            96       147   +51     
Impacted Files Coverage Δ
src/to-have-text-content.js 100.00% <100.00%> (ø)
src/utils.js 100.00% <0.00%> (ø)
src/to-be-empty.js 100.00% <0.00%> (ø)
src/to-have-prop.js 100.00% <0.00%> (ø)
src/to-have-style.js 100.00% <0.00%> (ø)
src/to-be-disabled.js 100.00% <0.00%> (ø)
src/to-contain-element.js 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3cfd279...4f2d3d3. Read the comment docs.

@brunohkbx
Copy link
Collaborator

@jeremyshearer thanks for opening this PR. Looks good, I just left a comment.

@Grohden
Copy link
Contributor

Grohden commented Jun 18, 2021

I was about to comment in #52 that it didn't worked for interpolated variables, but you've already fixed it XD

I'm not sure if it helps, but this change works for me :D

@jeremyshearer
Copy link
Contributor Author

@brunohkbx Thanks for taking a look! I have applied your excellent suggestion. Please let me know if you see anything else that needs to happen to get this merged in.

@brunohkbx
Copy link
Collaborator

Hi @jeremyshearer. I'm afraid we won't be able to merge this until we move from travis to github actions
#51

I'll try to take a look at it today and starts a draft

@brunohkbx brunohkbx merged commit 1bd7419 into testing-library:main Aug 3, 2021
@github-actions
Copy link

github-actions bot commented Aug 3, 2021

🎉 This PR is included in version 4.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants