Extract the resolving of a page's path and test it

[benstepp]

• Extracts the resolving of a page's path to it's own file for
  testability.
• Adds ava as a development dependency to write and run tests.
• Adds ava related settings to inherit the babel config and run the
  tests with npm test to package.json
• Refactor the quad-nested if statements in path resolving.

Working toward resolving Issue #154 one step at a time.

Not too sure on code style, so I didn't want to start adding es6 classes when there were none in the project. I opted for a closure with named functions to provide some readability instead of a super-if statement. The named functions also open up a nice readable place to fix user defined paths in frontmatter (#223) and allow for absolute/relative paths defined in frontmatter(#221).

Also not too sure if the gatsby team has any testing preferences on how to organize tests / readable descriptions for tests. I just matched the lib file structure in the test directory. First time using ava (well second. we tried it on an internal project at work and it started to choke around 100+ test files, but the ava team is working on fixing that).

Let me know if you have any feedback/review.

[KyleAMathews]

Wow! This is fantastic. A great refactor + tests! Yeah I don't really like classes generally. The approach you came up with is very readable and should be easy to maintain/extend going forward.

I don't have strong feelings about tests structure so what you've setup sounds great.

@gatsbyjs/gatsby-core-maintainers any thoughts?

[thangngoc89]

+1 for using AVA as test runner. Although be aware that AVA will probably break on old node (<5) if there are too many files

[benstepp]

I had some time tonight and split up glob pages to the point where I think it can be unit tested effectively. Do you guys see any problems with the way that I split it up? Would you have split it up differently?

Renamed the pathResolver from the first commit to urlResolver now that I have a clearer idea of what was happening in glob-pages. 100+ line functions really take a lot of cognitive load to fully understand and follow effectively. I can still build the blog-starter, so I don't think that I've broken anything. It's always hard to tell though without tests.

On a side note I just noticed while looking for some documentation that ava is changing the ok/notok to falsey/truthy in the very near future, so some minor refactoring will be required.

[KyleAMathews]

@benstepp really awesome stuff! I didn't have time to get it a full review but things looked good on my first path through. I'll give this a through review this Saturday (along with the other open PRs). And 100% agree glob-pages needed broken up.

[scottnonnenberg]

I love that we're bringing tests into the project! I have felt the pain of making changes in gatsby without them. However, unit tests are always a bit scary by themselves, so we now have a false sense of security about the whole project if npm test succeeds.

Would you consider adding a basic overall test scenario? Maybe just a couple files, run gatsby, validate that it runs successfully, puts files on disk?

[KyleAMathews]

@scottnonnenberg I totally agree e2e tests are needed. I have a number of ideas toward that end in #154. If @benstepp (or you) want to tackle that hallelujah but I think @benstepp meant this PR to be an incremental step along the way to full test coverage :-)

[benstepp]

#226 would be a good place to start. Adding them as submodules / test-fixtures could integrate them into the test suite. This does add a lot of developer overhead though, as the dependency count is pretty big, and we would have to keep them up to date.

Not sure if you would want to do a full integration test with actually calling the cli in shell, or abstract the cli from gatsby so that all it does is call a function to build and we would test the build function in the suite.

Also I don't think ava has test-tagging yet. With RSpec (ruby land, super strong) you could tag a test as slow and then skip running them, but I'm sure we could come up with something. Maybe a naming convention with test files and then two package.json scripts to target specifically named files and run all tests or the non-slow tests.

And yes, the PR is meant as an incremental step. Writing the first tests for a project are always hard, and you would usually test from the outside-in (integration into unit tests), but I felt like that would require too much reorganization to pull off quickly.

[KyleAMathews]

New Ava is out https://github.com/sindresorhus/ava/releases/tag/v0.14.0

[KyleAMathews]

@benstepp fantastic work! This is a really great start to getting Gatsby's testing story going! Thanks for all your contributions so far. I added you as a contributor to the repo btw :-)

[KyleAMathews]

I did need to make one minor fix to your refactoring as metadata from pages weren't being used in finding the path or returned correctly. 0ce3be5

[KyleAMathews]

I also added a eslint plugin for linting Ava which seems pretty cool 5ab7a0c https://github.com/sindresorhus/eslint-plugin-ava

[jlengstorf]

Hiya @benstepp! 👋

This is definitely late, but on behalf of the entire Gatsby community, I wanted to say thank you for being here.

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. (We’ve got t-shirts and hats, plus some socks that are really razzing our berries right now.)
2. If you’re not already part of it, we just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. You’ll receive an email shortly asking you to confirm. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If you have questions, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again! 💪💜