Skip to content
This repository has been archived by the owner on Dec 26, 2024. It is now read-only.

Rewrite [ButtonGroup] test #53

Merged
merged 1 commit into from
Aug 8, 2021
Merged

Rewrite [ButtonGroup] test #53

merged 1 commit into from
Aug 8, 2021

Conversation

yujonglee
Copy link
Contributor

@yujonglee yujonglee commented Aug 8, 2021

Description

I'm not expert of testing, so maybe I'm wrong with some parts.
Anyone wants to know my testing style in React, see my recent toy project.

  1. I left basic snapshoot testing and add tests below it. Because I don't to make radical changes. Plus, there might be something that only snap shot testing can catch.
  2. Using testID should considered as last option. Testing should be close to actual user.
    See Guiding Principles and How Should I Query?.

Actually, as I know, getByRole is number 1 priority in React-Testing-Libary. But it said getByText is number 1. I guess it's because getByRole is not that useful in RN(compare to React).

For example, code below is used lot more than Button. But below code can't be queried with getByRole('button').

<TouchableOpacity>
  <Text/>
<TouchableOpacity/>
  1. Test codes should be self-explainary. It should act like manual.
  2. Test case should be isolated. Shared fixture among test cases should be avoided. But sometimes, it is needed to remove duplicates. Than, it should be carefully handled, using const, beforeEach, mockClear, and given(it is not used here.)
  3. Rename event handler. As I know, it's idiomatic in React.
    See this article.

I will write some testing guide in the future. So maybe this is enough here.

Related Issues

I mentioned about snapshot testing in #42.

Tests

I rewrite existing test.

Help / Discussion

  1. There's no toHaveStyle property in JestMatchers<ReactTestInstance> warning.
    I think It's related to type. But I install @testing-library/jest-native, which includes type(as I know).
    I don't know how to solve this.

  2. Why createTestProps is needed?
    I found that (in this case) test doesn't breaks without it.
    I don't know lot about React Native. So if you give me some context or explanation, I'll be thankful.

  3. Why createComponent is needed?
    I think calling createComponent and wrap with ThemeProvider has very little difference in code size, but later is more clear and flexible as I did here.

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • Run yarn test or yarn test -u if you need to update snapshot.
  • Run yarn lint
  • I am willing to follow-up on review comments in a timely manner.

As I mentioned in #42 (comment), relying on snapshoot testing is not good practice.

But I don't want to make radical changes. So I left basic snapshoot testing and add tests below it.
@codecov
Copy link

codecov bot commented Aug 8, 2021

Codecov Report

Merging #53 (b4c51a2) into master (d9721ba) will decrease coverage by 0.18%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
- Coverage   94.09%   93.91%   -0.19%     
==========================================
  Files          21       21              
  Lines         542      542              
  Branches      231      231              
==========================================
- Hits          510      509       -1     
- Misses         32       33       +1     

@hyochan hyochan added the 🧪 test Issue or pr related to testing label Aug 8, 2021
Copy link
Owner

@hyochan hyochan left a comment

Choose a reason for hiding this comment

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

Far more readable tests!! Very cool things to have 🚀

Great work!

For the discussion on current PR, I'll move to discussion tab and notify you.

@hyochan hyochan merged commit 30d9491 into hyochan:master Aug 8, 2021
@hyochan hyochan linked an issue Aug 8, 2021 that may be closed by this pull request
@yujonglee yujonglee mentioned this pull request Aug 12, 2021
4 tasks
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
👷‍♂️ refactor 🧪 test Issue or pr related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Useless switch statement
2 participants