Skip to content

Commit

Permalink
Code review updates
Browse files Browse the repository at this point in the history
  • Loading branch information
benmccann committed Feb 20, 2019
1 parent bfc55fa commit 9fc7b57
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions docs/developers/contributing.md
Expand Up @@ -44,14 +44,14 @@ More information can be found in [gulpfile.js](https://github.com/chartjs/Chart.

### Image-Based Tests

Some display-related functionality is difficult to test via typical Jasmine unit test asserts. For this functionality, we have image-based tests that assert that the chart is drawn pixel-for-pixel matching an expected image.
Some display-related functionality is difficult to test via typical Jasmine units. For this reason, we introduced image-based tests ([#3988](https://github.com/chartjs/Chart.js/pull/3988) and [#5777](https://github.com/chartjs/Chart.js/pull/5777)) that assert that a chart is drawn pixel-for-pixel matching an expected image.

Generated charts in image-based tests should be **as minimal as possible** and focus only on tested features to prevent failure if another feature breaks (e.g. disable the title and legend when testing scales).
Generated charts in image-based tests should be **as minimal as possible** and focus only on the tested feature to prevent failure if another feature breaks (e.g. disable the title and legend when testing scales).

You can create a new image-based test by following the steps below:
- Create a JS file ([example](https://github.com/chartjs/Chart.js/blob/f7b671006a86201808402c3b6fe2054fe834fd4a/test/fixtures/controller.bubble/radius-scriptable.js)) or JSON file ([example](/chartjs/Chart.js/blob/4b421a50bfa17f73ac7aa8db7d077e674dbc148d/test/fixtures/plugin.filler/fill-line-dataset.json)) that defines chart config and generation options.
- Add this file in`test/fixtures/{spec.name}/{feature-name}.json`.
- Add a [describe line](/chartjs/Chart.js/blob/4b421a50bfa17f73ac7aa8db7d077e674dbc148d/test/specs/plugin.filler.tests.js#L10) to the beginning of `specs/{spec.name}.js`.
- Create a JS file ([example](https://github.com/chartjs/Chart.js/blob/f7b671006a86201808402c3b6fe2054fe834fd4a/test/fixtures/controller.bubble/radius-scriptable.js)) or JSON file ([example](https://github.com/chartjs/Chart.js/blob/4b421a50bfa17f73ac7aa8db7d077e674dbc148d/test/fixtures/plugin.filler/fill-line-dataset.json)) that defines chart config and generation options.
- Add this file in `test/fixtures/{spec.name}/{feature-name}.json`.
- Add a [describe line](https://github.com//chartjs/Chart.js/blob/4b421a50bfa17f73ac7aa8db7d077e674dbc148d/test/specs/plugin.filler.tests.js#L10) to the beginning of `specs/{spec.name}.js` if it doesn't exist yet.
- Set `"debug": true` in the JSON file to prevent the canvas destruction when running tests.
- Run `gulp unittest --watch --inputs=test/specs/{spec.name}.js`.
- Click the *"Debug"* button (top/right): a test should fail with the associated canvas visible.
Expand All @@ -60,7 +60,7 @@ You can create a new image-based test by following the steps below:
- Verify test relevancy by changing the feature values *slightly* in the JSON file.
- Remove `debug: true` from the JSON file.

Tests should pass in both browsers. In general, we've hidden the text for all the image tests since it's quite difficult to get them passing between different browsers. As a result, it is recommended to hide all scales in image-based tests. It is also recommended to disable animations in image-based tests. If tests still do not pass, adjust [`tolerance` and/or `threshold`](https://github.com/chartjs/Chart.js/blob/1ca0ffb5d5b6c2072176fd36fa85a58c483aa434/test/jasmine.matchers.js) at the beginning of the JSON file keeping them **as low as possible**.
Tests should pass in both browsers. In general, we've hidden the text for all the image tests since it's quite difficult to get them passing between different browsers. As a result, it is recommended to hide all scales in image-based tests. It is also recommended to disable animations. If tests still do not pass, adjust [`tolerance` and/or `threshold`](https://github.com/chartjs/Chart.js/blob/1ca0ffb5d5b6c2072176fd36fa85a58c483aa434/test/jasmine.matchers.js) at the beginning of the JSON file keeping them **as low as possible**.

## Bugs and Issues

Expand Down

0 comments on commit 9fc7b57

Please sign in to comment.