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

Issue 737: add jest support for running tests, add simple first test #738

Closed
wants to merge 1 commit into from

Conversation

schambers
Copy link
Contributor

I noticed there are no frontend tests for any of the react components. I thought it may be worthwhile to introduce jest and add a very simple first test.

It was required to mock src/lib/misc.ts as this module inits all of online-go.com and is prohibitive of running tests in isolation. Thankfully mocking out this one component property runs the test.

We may want to leverage enzyme for testing snapshots of the DOM but I've never used it before and wanted to submit this first to see if the OGS team wanted to go in this direction.

Fixes #737

@anoek
Copy link
Member

anoek commented Jan 22, 2019

I'd love to integrate a test framework :)

I'm not familiar enough with the options myself, so before I merge anything I'd like to see a good set of tests developed to make sure that whatever we pick is going to be effective for our case.

@schambers
Copy link
Contributor Author

I can add a couple more tests. I'm admittedly new to jest myself.

One challenge I ran into was the module loading is all tied to some files that inherently load the Dom/inject script tags. I managed to mock out the needed pieces but I didn't want to introduce any refactorings as part of using the test framework. This issue showed a very simple test just to show that it can be done. I can create more tests when I have a chance to show that it would provide value.

I think it would be best to add them as part of this one issue and I can send another pull request.

@anoek
Copy link
Member

anoek commented Feb 19, 2019

Feel free to re-open if you want to pick this back up

@anoek anoek closed this Feb 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants