Adds more tests for untested services

[scottwarren]

Related Issue
#41 @joshwcomeau requested some tests to be written for the more important files/functions

Summary

Adding tests for some of the services/modules:

• Project Name Service
• Project Type Specifics

For the Project Name service, I had to export the prefixes and suffixes array to test against this list given the format we output. Let me know if you guys think this is an appropriate way to test this.

I did also add some tests for the create-project.service.js, but it doesn't test all of the contained functions (just the easy ones 😝). I did have to mock a few things, but maybe this mocking should live somewhere else, instead. WDYT?

There was a test that was failing for me locally in the dependencies.reducer.test.js, so I updated it. If that's not okay, I can revert that change.

[joshwcomeau]

This looks great! Thanks for adding these.

I'm requesting changes for a couple small things, described in the inline comments.

Also, it looks like the file project-type-specifics.test.js isn't being transformed by Prettier - I see different quotes used, lines that are too long, etc. There's a pre-commit hook that should run Prettier, so it should be impossible for PRs to not be formatted :/ do you have any clue why it wasn't formatted?

[joshwcomeau]

Yay thanks for thinking to test this!

For someone checking out this file, it might not be clear why this array is being exported. Could you please add a comment like "Exported so that getColorForProject can be tested"?

[scottwarren]

Yeah, that's a good idea. I've added the comment now.

[joshwcomeau]

Thanks!

[joshwcomeau]

Yeah, I think this is fair!

[joshwcomeau]

I wonder if these tests are too specific...

What I mean by that is, the getDocumentationLink function doesn't do any work, it just returns a string, and by checking that exact string, the tests will fail when we update the documentation link. I can't think of a lot of reasons that the failure would be genuine (since there's no real room for someone to make an error inside the method).

I like the final clause, checking that it throws. But maybe we can just check that both values are strings, and they aren't equal?

it("provides strings for Gatsby and CRA", () => {
  const gatsbyString = getDocumentationLink('gatsby');
  const createReactAppString = getDocumentationLink('create-react-app');

  expect(typeof gatsbyString).toEqual('string');
  expect(typeof createReactAppString).toEqual('string');

  expect(gatsbyString).not.toBe(createReactAppString);
}

I know some feel like it's bad form to have multiple expects per test. I'm happy either way: you could create 3 it statements for these checks if you prefer!

[scottwarren]

I wonder if these tests are too specific...

What I mean by that is, the getDocumentationLink function doesn't do any work, it just returns a string, and by checking that exact string, the tests will fail when we update the documentation link. I can't think of a lot of reasons that the failure would be genuine (since there's no real room for someone to make an error inside the method).

I like the final clause, checking that it throws. But maybe we can just check that both values are strings, and they aren't equal?

I think you're right, there is a level of too specific/too fine-grained with tests where the test becomes unhelpful and counter-productive, and I think this test is passed that line. This is fixed up now!

[scottwarren]

This looks great! Thanks for adding these.

I'm requesting changes for a couple small things, described in the inline comments.

Thanks for the review! All of these are fixed 👍

Also, it looks like the file project-type-specifics.test.js isn't being transformed by Prettier - I see different quotes used, lines that are too long, etc. There's a pre-commit hook that should run Prettier, so it should be impossible for PRs to not be formatted :/ do you have any clue why it wasn't formatted?

It is possible to bypass them commit hooks by running git commit --no-verify, but IDK if that's why was that file didn't have Prettier run over it. I have made sure Prettier has been run over the file now though.

[joshwcomeau]

Looks great! Thanks for contributing this :D

[joshwcomeau]

Thanks!

[joshwcomeau]

Thanks for changing this :D I expect those links might change (ideally we'd have our own docs describing them, since the current docs assume CLI know-how), so it'll be nice to not have to update the tests.

[joshwcomeau]

Hey @scottwarren, I just sent an invite to become a collaborator on Guppy :D I really appreciate your contributions to increasing our test coverage.

You can learn more about how we view collaborators in the Contributing docs. Please give it a quick read, and let me know if you have any questions :D

Thanks again for your contributions!

[scottwarren]

Thanks @joshwcomeau, I am glad to help!