update service tests tutorial, affects [wercker] also [dependabot docker dynamic-json dynamic-xml dynamic-yaml npm website] #2075
Conversation
Generated by 🚫 dangerJS |
|
I might end up needing to update this a bit in light of #2076 . If so, I will, but I think the broad gist of the docs will remain the same so comments on this will still be helpful :) |
|
Thanks for review. I'm going to leave this one open for the moment and review the examples once #2103 is done |
resolve conflicts on wercker.tester.js
|
This should be ready for a second look now. I think at some point I'd like to find another badge to use as a worked example in the docs as I think the changes in #2103 mean this is now not the best example. Simultaneously, I also don't want to make perfect the enemy of the good. Lets get the docs up-to-date as a first step. As we refactor more badges, I'll find another better example and we can revisit. |
doc/service-tests.md
Outdated
| runner handles that for us. | ||
| 4. `Joi.object().keys()` defines a Joi object schema containing some defined keys | ||
| 5. We expect `name` to be a string literal `"build"` | ||
| 6. Because this test depends on a live service, we don't want our test to depend on our API call returning a particular build status. Instead we should perform a "picture check" to assert that the badge data conforms to an expected pattern. Our test should not depend on the status of the example project's build, but should fail if trying to generate the badge throws an error, or if there is a breaking change to the upstream API. In this case we will use a pre-defined regular expression to check that the badge value looks like a build status. [services/test-validators.js](https://github.com/badges/shields/blob/master/services/test-validators.js) defines a number of useful validators we can use. Many of the common badge types (version, downloads, rank, etc) already have validators defined here. |
There was a problem hiding this comment.
Shouldn't be etc. instead of etc?
| 2. We use the `get()` method to request a badge. There are several points to consider here: | ||
| * We need a real project to test against. In this case we have used [wercker/go-wercker-api](https://app.wercker.com/wercker/go-wercker-api/runs) but we could have chosen any stable project. | ||
| * Note that when we call our badge, we are allowing it to communicate with an external service without mocking the reponse. We write tests which interact with external services, which is unusual practice in unit testing. We do this because one of the purposes of service tests is to notify us if a badge has broken due to an upstream API change. For this reason it is important for at least one test to call the live API without mocking the interaction. | ||
| * All badges on shields can be requested in a number of formats. As well as calling https://img.shields.io/wercker/build/wercker/go-wercker-api.svg to generate  we can also call https://img.shields.io/wercker/build/wercker/go-wercker-api.json to request the same content as JSON. When writing service tests, we request the badge in JSON format so it is easier to make assertions about the content. |
There was a problem hiding this comment.
Currently https://img.shields.io/wercker/build/wercker/go-wercker-api.svg returns 404.
There was a problem hiding this comment.
It does, but that's because I updated this doc based on #2103 and #2103 (comment) isn't deployed yet. Once that's live, that example will render. If you want, we can block merging this until the next deploy.
There was a problem hiding this comment.
We shouldn't wait for the next deploy. This tutorial is great and can go live without this badge in my opinion.
|
|
||
| 1 passing (1s) | ||
| ``` | ||
|
|
||
| That's looking good! | ||
|
|
||
| Next we'll add a second test for a branch build. | ||
| Sometimes if we have a failing test, it is useful to be able to see some logging output to help work out why the test is failing. We can do that by calling `npm run test:services:trace`. Try running |
There was a problem hiding this comment.
What do you think about adding a info how to run one test? I use this command: npm run test:services -- --only=wercker --fgrep="Build passed (mocked)"
There was a problem hiding this comment.
I never knew this existed until today. Every day's a school day 🏫
doc/service-tests.md
Outdated
| and path. | ||
|
|
||
| [icedfrisby-nock]: https://github.com/paulmelnikow/icedfrisby-nock#usage | ||
| [Nock]: https://github.com/node-nock/nock | ||
|
|
||
| Sometimes it may be also helpful to include service tests which receive a known value from the API. For example, in the `render()` method of our service, there is some logic which sets the badge color based on the build status: |
There was a problem hiding this comment.
In my opinion every service should have a test checking a specific value and in most cases it can be achieved by mocking a response. Test validators are checking if value has one of allowed values and I can image a build badge which returns always failing or a download badge which returns always 1 and all tests are passing.
There was a problem hiding this comment.
I can see you addressed this issue. Thanks!
doc/service-tests.md
Outdated
| .get('/wercker/go-wercker-api/builds?limit=1') | ||
| .reply(200, [{ status: 'finished', result: 'passed' }]) | ||
| ) | ||
| .expectJSON({ name: 'build', value: 'passing', colorB: '#4c1' }) |
There was a problem hiding this comment.
Service returns brightgreen in service code, but here we compare result with #4c1. What do you think about this approach?
There was a problem hiding this comment.
Aah yes. I skipped over this when I first wrote it because I found the boilerplate a bit clunky and I didn't want to document it that way. Its good you raised it though. Its a bit of a tangent but I've extracted that boilerplate in a9c67ea just to make this a bit nicer to use moving forwards.
- etc --> etc. - add note about running one test with --fgrep - test suite should always include test with known/mock values - use named colours in colour testing example
chris48s
changed the title
chris48s
changed the title
|
Thanks for getting these reviews done this evening 👍 |
I've gone over the service tests tutorial and updated it so that we work through writing tests for a service which extends
BaseJsonService, reflecting our current testing practice.Wercker seemed like a good example to use - it allowed me to cover the things I wanted to cover without being too complicated. The tests I've added for wercker aren't massively useful but since I put them in the tutorial we might as well add them to the test suite :)
refs #1984