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

Lint and run tests as part of yarn prepare to improve safety #2038

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

hsource
Copy link
Contributor

@hsource hsource commented Oct 11, 2022

Motivation

While trying to work on this repository, I realized that the linting and tests were broken. To help ensure the tests and linting isn't broken by any new changes, I think it'd be great to run the linter and tests as part of the prepare release process.

Fix

  1. Change yarn prepare to also run yarn lint && yarn test
    • Create a separate yarn build command so we can build while skipping the lint/test process
  2. Fix the test by moving the mock to the outermost scope
  3. Fix the linting by adding excludes to the typechecker

Testing

Ran yarn prepare and verified that it runs successfully.

@andresesfm
Copy link
Contributor

Hello @hsource thank you for the contribution. Can you please describe how this change "improves safety". Also if you move the mock outside of describe how would you ad ios tests?

@andresesfm andresesfm merged commit 8091c5f into hyochan:main Oct 11, 2022
@hsource
Copy link
Contributor Author

hsource commented Oct 12, 2022

@andresesfm

Hello @hsource thank you for the contribution. Can you please describe how this change "improves safety".

Thanks for the questions! What I meant by this was that the typechecking (yarn lint:tsc) and tests (yarn test) seemed to have errors that weren't caught by CI.

Thinking about this more after your comment, I think we can revert this change and change prepare back to just run bob build again. Instead, we may want to just add yarn test to Github CI pipelines.

Also if you move the mock outside of describe how would you ad ios tests?

Right now, there aren't any iOS tests, so it's not a problem. jest.mock only seemed to work correctly at the top-level.

I think if we had iOS tests, I'd recommend renaming iap.test.ts to iap.android.test.ts and creating a separate iap.ios.test.ts that tests iOS logic.

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