From 85f97a42d09c9ca7143642910b9969a977b6117d Mon Sep 17 00:00:00 2001 From: Bryan Kendall Date: Mon, 14 Dec 2015 19:25:25 -0800 Subject: [PATCH 1/6] code format and testing I attempted to bring in all the thoughts on #1 and #2. Let's finish these off and get them into something official looking! --- README.md | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 102 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 51af339..e1fa4a5 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,102 @@ -# styleguide -contains runnable coding guidelines to maintain consistency across all our repos +# Runnable Code Style Guide + +Runnable's approach to writing code and tests. + +## Table of Contents + +1. [Code Formatting](#code-formatting) +1. [Testing](#testing) + 1. [Types of Tests](#types-of-tests) + 1. [Stubbing](#stubbing) + 1. [Asserting](#asserting) + 1. [Additional Testing Reading](#additional-testing-reading) + +## Code Formatting + +[![js-standard-style](https://cdn.rawgit.com/feross/standard/master/badge.svg)](https://github.com/feross/standard) + +We use [`standard`](http://standardjs.com/) in most projects. If it passes `standard`, anything else is up to the developer. No questions asked. + +**[back to the top](#table-of-contents)** + +## Testing + +Let's discuss various tests, what they should do and cover, and how best to organize them. + +### Types of Tests + +There are three types of tests we should be concerned about. + +- Unit +- Integration +- Functional + +#### Definitions + +**Unit** tests are pretty obvious (or should be). They test exactly one unit of functionality. Most likely this should follow the boundaries of functions or at largest, modules. If we're finding we're writing a lot unit tests for a specific function, it may need to be broken up. + +An important note: unit tests should _never_ touch an external service (e.g. database). + +**Integration** tests make sure that one or more components function correctly together. This can be multiple functions within a module, multiple modules interacting with each other, or a module interacting with a database or external service. + +**Functional** tests should align with product ideas. If a user goes through a given flow, they should finish with some outcome. We've found these tests to be pretty difficult to work with as of late, and are trying to get rid of some of these. + +#### Organization + +In most cases, we should attempt to organize our file structure as follows: + +``` +repository/ + lib/ (or something like it) + tests/ + unit/ + integration/ + functional/ +``` + +Additionally, the `package.json` should support the following commands: + +- `npm test`: runs a code linter and all three sets of tests +- `npm unit`: runs the unit tests +- `npm integration`: runs the integration tests +- `npm functional`: runs the functional tests + +### Stubbing + +One of the most powerful tools we have (and are still learning to use well) is `sinon`. `sinon` is a stubbing and spying library that helps us assert that functions are called with what we expect. + +`sinon`'s [docs](http://sinonjs.org/docs/) are quite good, but we will talk about some guidelines to follow when using it: + +- In all tests where we are going to stub or spy on something, do so in a `before*` +- In all tests where we do stub or spy on something, + - if it is on an object that is regenerated every test, no action is necessary + - if it is on a module or something persistent, always restore the function in an `after*` +- Stub modules and functions in the correct scopes (meaning, as close to the test that uses it as possible, but hierarchal stubbing is fine) +- When stubbing functions, we may stub the function to a 'truthy' behavior; doing so reduces code duplication, though the developer will need to adjust the stub(s) to test error cases +- Collecting stubs into logical blocks and using the language to define what is being set up is encouraged (i.e. use `describe` blocks to collect and describe stubs, `it` blocks to say what happens) + +### Asserting + +Now that we have tests all squared away and written, let's talk about our assertions. + +First, a few options we have been using are `code` and `chai.assert`. [`code`](https://github.com/hapijs/code) is pretty straight forward, using the `expect` form of assertions. [`chai`](https://github.com/chaijs/chai)'s assertion library has worked well, especially with [promises](https://github.com/domenic/chai-as-promised), and we have been using chai's `assert` which helps keep straight which library with which we are working. + +We also have started using `sinon`'s assert features as well (e.g. `sinon.assert.calledWith`) which provides much nicer error messages than alternatives. + +A few notes to help guide assertions: + +- All tests must assert something +- When testing with a stub in the same logical group, we must assert something about the stub +- Stubs should (most likely) include two checks from this list: + - `sinon.assert.called` + - `sinon.assert.notCalled` + - `sinon.assert.calledOnce`, `sinon.assert.calledTwice`, etc. + - `sinon.assert.calledWith` + - `sinon.assert.calledWithExactly` (preferred over `calledWith`) +- When testing with promises (and `chai-as-promised`), always start with something such as `return assert.isFulfilled(myPromise())`, and always remember to return your promise. + +### Additional Testing Reading + +An interesting read on how 'value' could be perceived in testing is one of the posts on [Google's Testing Blog](http://googletesting.blogspot.com/2015/04/just-say-no-to-more-end-to-end-tests.html). It's not super long, so I encourage anyone to read. They very much encourage unit and integration tests over end-to-end (I read it as functional) tests as they help the developers much more and have the exact same value to the customer as any end-to-end test: a bug fix. If you can write a unit test for your bug rather than an end-to-end test, wouldn't you rather do that? + +**[back to the top](#table-of-contents)** From 9d8bf130764e30eafe23f2f725d1dd5b683b6b9a Mon Sep 17 00:00:00 2001 From: Bryan Kendall Date: Tue, 15 Dec 2015 09:53:16 -0800 Subject: [PATCH 2/6] change title to patterns --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e1fa4a5..b9f5e6a 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# Runnable Code Style Guide +# Runnable Code and Test Patterns Guide Runnable's approach to writing code and tests. From 93369865a2fb0a609e700c5ee0d00bb408469529 Mon Sep 17 00:00:00 2001 From: Bryan Kendall Date: Tue, 15 Dec 2015 13:46:53 -0800 Subject: [PATCH 3/6] fix misspelling hierarchical is hard, okay? --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b9f5e6a..215c0a2 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ One of the most powerful tools we have (and are still learning to use well) is ` - In all tests where we do stub or spy on something, - if it is on an object that is regenerated every test, no action is necessary - if it is on a module or something persistent, always restore the function in an `after*` -- Stub modules and functions in the correct scopes (meaning, as close to the test that uses it as possible, but hierarchal stubbing is fine) +- Stub modules and functions in the correct scopes (meaning, as close to the test that uses it as possible, but hierarchical stubbing is fine) - When stubbing functions, we may stub the function to a 'truthy' behavior; doing so reduces code duplication, though the developer will need to adjust the stub(s) to test error cases - Collecting stubs into logical blocks and using the language to define what is being set up is encouraged (i.e. use `describe` blocks to collect and describe stubs, `it` blocks to say what happens) From 222d21132f1b616552b4990c5374df40c299ac33 Mon Sep 17 00:00:00 2001 From: Bryan Kendall Date: Tue, 15 Dec 2015 13:49:17 -0800 Subject: [PATCH 4/6] more specific stub assertions --- README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 215c0a2..e22e018 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,9 @@ We also have started using `sinon`'s assert features as well (e.g. `sinon.assert A few notes to help guide assertions: - All tests must assert something -- When testing with a stub in the same logical group, we must assert something about the stub +- When testing with a stub in the same logical group, we must assert something about the stub in at least one test. Should include + - if it was called, or not + - what it was called with, if applicable - Stubs should (most likely) include two checks from this list: - `sinon.assert.called` - `sinon.assert.notCalled` From 39cb3829b960e8c6bc30b39f7e422ceb8ae183f4 Mon Sep 17 00:00:00 2001 From: Bryan Kendall Date: Fri, 18 Dec 2015 17:46:44 -0800 Subject: [PATCH 5/6] add `callOrder` --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e22e018..5f06352 100644 --- a/README.md +++ b/README.md @@ -89,10 +89,11 @@ A few notes to help guide assertions: - When testing with a stub in the same logical group, we must assert something about the stub in at least one test. Should include - if it was called, or not - what it was called with, if applicable -- Stubs should (most likely) include two checks from this list: +- Stubs should (most likely) include checks from this list: - `sinon.assert.called` - `sinon.assert.notCalled` - `sinon.assert.calledOnce`, `sinon.assert.calledTwice`, etc. + - `sinon.assert.callOrder` - `sinon.assert.calledWith` - `sinon.assert.calledWithExactly` (preferred over `calledWith`) - When testing with promises (and `chai-as-promised`), always start with something such as `return assert.isFulfilled(myPromise())`, and always remember to return your promise. From a0e5791d7d1706bfd39c4af7f4b47eb4c542d534 Mon Sep 17 00:00:00 2001 From: Bryan Kendall Date: Fri, 18 Dec 2015 17:51:39 -0800 Subject: [PATCH 6/6] expand on test categories --- README.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/README.md b/README.md index 5f06352..56ad3f4 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,12 @@ An important note: unit tests should _never_ touch an external service (e.g. dat **Functional** tests should align with product ideas. If a user goes through a given flow, they should finish with some outcome. We've found these tests to be pretty difficult to work with as of late, and are trying to get rid of some of these. +##### Where do you draw the line? + +There will always be discussion on where the lines between (for example) **unit** and **integration** tests. For a unit test in a mongoose environment, mocking out the layer right at mongoose and simply returning sample objects is sufficient. The developer should have faith (and understanding) that mongoose will correctly return objects. If there's more paculiar logic that you may not be sure of or is more complex (like a map-reduce or aggrigation in mongo, or making sure populate is doing the correct thing), you would move to an **integration** test and write it there. + +These definitely aren't hard-and-fast lines in the sand. A good goal would be that no external services are required to get unit tests off the ground and running, but integration tests may need more setup (like a database). When we find various tests that don't fit into the classification they're housed in, we can always fix the dependency or move the test when appropriate. + #### Organization In most cases, we should attempt to organize our file structure as follows: