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.

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

Misc improvements to tests #67

Merged
merged 1 commit into from
Aug 11, 2018
Merged

Conversation

gavinorland
Copy link
Collaborator

Addresses #57 and implements:

  • Use of actual component name throughout tests rather than "Component"
  • Improvements to smoke tests, using exists()
  • Improvements to test descriptions
  • Improvements to test divisions (more describe blocks)
  • More separation between tests (almost always only one expect() per test and wrappers usually include only tested props)
  • New wrapper instances used per test, rarely using setProps()
  • Truly shared data established at top of file, outside describe blocks
  • No re-use or resetting of mocks between tests, but creation of new ones
  • Some style checks removed when this is testing Emotion rather than our components (props/conditionals)
  • Appropriate use of mount and shallow
  • Thorough snapshot tests (using mount and full range of props)

Coverage remains at 100%.

@gavinorland gavinorland requested a review from stevesims August 10, 2018 08:30
@gavinorland
Copy link
Collaborator Author

Will address this snapshot issue causing build failure - seen this before!

@codecov
Copy link

codecov bot commented Aug 10, 2018

Codecov Report

Merging #67 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #67   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          22     22           
  Lines         183    183           
  Branches       39     39           
=====================================
  Hits          183    183
Impacted Files Coverage Δ
components/array-object-table/src/index.js 100% <ø> (ø) ⬆️
components/table/src/index.js 100% <ø> (ø) ⬆️

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 bccb1ae...3ce053f. Read the comment docs.

@gavinorland
Copy link
Collaborator Author

@stevesims All now passing.

});

Copy link
Owner

Choose a reason for hiding this comment

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

hmm - whitespace on an empty line should i believe be a linting error

});

Copy link
Owner

Choose a reason for hiding this comment

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

another case of whitespace on an otherwise empty line...

stevesims
stevesims previously approved these changes Aug 10, 2018
Copy link
Owner

@stevesims stevesims left a comment

Choose a reason for hiding this comment

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

barring the whitespace things that i think should be linting errors ;P this is a 👍

@gavinorland gavinorland force-pushed the feature/improve-unit-tests branch 2 times, most recently from b8120cb to eea8793 Compare August 10, 2018 18:14
Fix snapshot discrepancy

n.b. Also adjust Jest config as per jestjs/jest#6766 (comment)

Remove whitespace

Minor fixes
@gavinorland gavinorland force-pushed the feature/improve-unit-tests branch from eea8793 to 3ce053f Compare August 10, 2018 18:40
@gavinorland
Copy link
Collaborator Author

Now up-to-date with master and good to go...

@gavinorland gavinorland merged commit d67461c into master Aug 11, 2018
@gavinorland gavinorland deleted the feature/improve-unit-tests branch August 11, 2018 09:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants