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

feat: jest tests #140

Merged
merged 11 commits into from Nov 19, 2021
Merged

feat: jest tests #140

merged 11 commits into from Nov 19, 2021

Conversation

diegotsi
Copy link
Contributor

Hey guys! Recently I needed to create some mocks for this lib, so I'm bringing this to the lib to help others that want to do the same.

These are the features from this PR:

1 - Configure Jest
2 - Create a mock folder with AppEventParams and AppEvents to make a more faithful mock, as this content came from the Native we can't access this with AppEventsLogger.AppEvents on Jest, that's why I'm putting this on the mocks folder.
3 - Update readme with a basic example module.

Screen Shot 2021-11-19 at 10 52 05

@diegotsi diegotsi changed the title Feat/jest tests feat: jest tests Nov 19, 2021
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

This is really cool! Such a common need. I had a few thoughts on various parts and left comments - curious what you think

package.json Show resolved Hide resolved
jest/setup.js Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

The CI addition is fantastic, and the single jest setup line is fantastic
I left a note on including it in the released module, that's important :-)
I'm still confused about the eslint integration - I just don't understand that. We don't have to block the PR on it but if I can't understand it I would at least like a comment in there that has the URL to this discussion and a note "this may not be necessary?"

So the package.json change is the blocker, the eslint thing I'd like to understand but could let it go

README.md Show resolved Hide resolved
jest/setup.js Show resolved Hide resolved
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

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

This is a thing of beauty, thank you!

https://github.com/thebergamo/react-native-fbsdk-next/runs/4269445913?check_suite_focus=true#step:8:10

$ jest
PASS RNFBSDKExample/__tests__/App-test.js
  ✓ renders correctly (717 ms)

@mikehardy mikehardy merged commit cc904e7 into thebergamo:master Nov 19, 2021
github-actions bot pushed a commit that referenced this pull request Nov 19, 2021
# [6.1.0](v6.0.0...v6.1.0) (2021-11-19)

### Features

* **test:** add jest tests and mock file for library users ([#140](#140)) ([cc904e7](cc904e7))
@github-actions
Copy link

🎉 This PR is included in version 6.1.0 🎉

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.

None yet

2 participants