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

Add unit tests #42

Merged
merged 12 commits into from
Feb 8, 2020
Merged

Add unit tests #42

merged 12 commits into from
Feb 8, 2020

Conversation

ricokahler
Copy link
Collaborator

@ricokahler ricokahler commented Feb 7, 2020

Per our discussion in #41 , here are some unit tests!

A few things to note:

  • As you previous stated, this really is just testing "the glue" and it's not 100% sufficient in seeing if everything works, more on this later.
  • I opted for GitHub actions instead of Travis because I think that's what people choose nowadays? Feel free to interject here but I think this is a good choice because it just works right in GitHub.
  • I added babel into the mix here because configuring Jest with TypeScript is easy via Babel. I'm not sure if there are big limitations in this approach but it works as of now.
  • I tested using javascript because I find mocking with TS to be challenging but I'm open to changing this
  • If you like these types of tests, it may be worth adding required status checks so that the tests pass before merging.

CI via Github Actions

See the result of my GitHub Actions in my fork here and see the configuration here

Coverage reporting

I also have some branches in my fork that do coverage reporting if you're into that. See here and here

Testing more than "the glue"

As stated above, these unit tests are not exactly sufficient to see if everything works. Now that this PR adds GitHub Actions, I think the best course of action is to create a new GitHub Action Workflow that runs a demo Gatsby build. I don't want to get too carried away in scope right now so I'll probably create a new issue for this. (edit: here it is)

Let me know what you think!

@d4rekanguok
Copy link
Owner

@ricokahler, thank you for this incredibly thorough PR, I didn't expect 100% coverage right off the bat! I've also learned more about writing tests from your code 🤯

We're not using fancy typescript-only features so using babel typescript is completely fine. I also don't think writing tests in .ts would add significant value.

Thanks again for this, merging now!

@d4rekanguok d4rekanguok merged commit 3482db3 into d4rekanguok:master Feb 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants