-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add documentation for regression tests #2000
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
b460e9d
Extract instructions for adding new regression tests
floehopper 782cf39
First attempt at documentation about regression tests
floehopper 2b2b45d
Link to regression tests doc from CONTRIBUTING.md
floehopper c072ded
Link to regression test doc from merging-content-prs doc
floehopper File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,3 +20,5 @@ | |
## Testing ## | ||
|
||
Write tests. | ||
|
||
Make sure the [regression tests](doc/regression-tests.md) are passing. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
# Adding new regression tests | ||
|
||
1. Update the flow to replace any single line conditionals around `Phraselist`s with multiple line conditionals. This is so that we get useful information from the running the coverage utility. Single line conditionals will show up as having been exercised irrespective of whether they caused something to be added to the `Phraselist`. | ||
|
||
# Replace single line conditional | ||
phrases << :new_phrase if condition | ||
|
||
# With multiple line alternative | ||
if condition | ||
phrases << :new_phrase | ||
end | ||
|
||
2. Generate a set of responses for the flow that you want to add regression tests to. | ||
|
||
$ rails r script/generate-questions-and-responses-for-smart-answer.rb \ | ||
<name-of-smart-answer> | ||
|
||
3. Commit the generated questions-and-responses.yml file (in test/data) to git. | ||
|
||
4. Change the file by adding/removing and changing the responses: | ||
|
||
* Add responses for any of the TODO items in the file. | ||
|
||
* Remove responses that you don't think cause the code to follow different branches, e.g. it might be useful to remove all but one (or a small number) of countries to avoid a combinatorial explosion of input responses. | ||
|
||
* Combine responses for checkbox questions where the effect of combining them doesn't affect the number of branches of the code that are exercised. | ||
|
||
5. Commit the updated questions-and-responses.yml file to git. | ||
|
||
6. Generate a set of input responses and expected results for the Smart Answer. | ||
|
||
$ rm -rf coverage && \ | ||
TEST_COVERAGE=true \ | ||
rails r script/generate-responses-and-expected-results-for-smart-answer.rb \ | ||
<name-of-smart-answer> | ||
|
||
7. Inspect the code coverage report for the Smart Answer under test (`open coverage/rcov/index.html` and find the smart answer under test). | ||
|
||
* If all the branches in the flow have been exercised then you don't need to do anything else at this time. | ||
|
||
* Code in node-level blocks (e.g. in `value_question`, `date_question`, `multiple_choice` & `outcome` blocks) will always be executed at *flow-definition-time*, and so coverage of these lines is of **no** significance when assessing test coverage of the flow logic. | ||
|
||
* Code in blocks inside node-level blocks (e.g. in `precalculate`, `next_node_calculation`, `validate` & `define_predicate` blocks) will be executed at *flow-execution-time*, and so coverage of these lines is of significance when assessing test coverage of the flow logic. | ||
|
||
* Coverage of code in ancillary classes (e.g. calculators) should also be considered at this point. | ||
|
||
* If there are branches in the flow that haven't been exercised then: | ||
|
||
* Determine the responses required to exercise those branches. | ||
|
||
* Go to Step 4, add the new responses and continue through the steps up to Step 7. | ||
|
||
8. Commit the generated responses-and-expected-results.yml file (in test/data) to git. | ||
|
||
9. Generate a yaml file containing the set of source files that this Smart Answer depends upon. The script will automatically take the ruby flow file, locale file and erb templates into account. You just need to supply it with the location of any additional files required by the Smart Answer (e.g. calculators and data files). This data is used to determine whether to run the regression tests based on whether the source files have changed. | ||
|
||
$ rails r script/generate-checksums-for-smart-answer.rb \ | ||
<name-of-smart-answer> \ | ||
<path/to/additional/files> | ||
|
||
10. Commit the generated yaml file to git. | ||
|
||
11. Run the regression test to generate the Govspeak of each landing page and outcome reached by the set of input responses. | ||
|
||
$ RUN_REGRESSION_TESTS=<name-of-smart-answer> \ | ||
ruby test/regression/smart_answers_regression_test.rb | ||
|
||
If you want individual tests to fail early when differences are detected, set `ASSERT_EACH_ARTEFACT=true`. | ||
Note that this more than doubles the time it takes to run regression tests. | ||
|
||
12. Commit the generated Govspeak files (in test/artefacts) to git. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
# Regression tests | ||
|
||
The project includes a set of regression tests. These tests are not *normally* run as part of the standard build, because they take a long time to run. You can run just the regression tests with the following command: | ||
|
||
$ RUN_REGRESSION_TESTS=true ruby test/regression/smart_answers_regression_test.rb | ||
|
||
You can run just the regression tests for a single flow using this command: | ||
|
||
$ RUN_REGRESSION_TESTS=<smart-answer-flow-name> ruby test/regression/smart_answers_regression_test.rb | ||
|
||
Note that the `RUN_REGRESSION_TESTS` environment variable can also be used in conjunction with the rake `test` task if you want to force regression tests to run as part of the standard build. | ||
|
||
By default most of the assertions in the regression tests are combined into a single assertion at the end. If you want the regression tests to fail fast then set `ASSERT_EACH_ARTEFACT=true`. However, you should note that this more than doubles the time it takes them to run. | ||
|
||
Running the test re-generates a set of HTML/Govspeak files in `test/artefacts` based on the files in `test/data`. | ||
|
||
## Test data | ||
|
||
* `<smart-answer-flow-name>-questions-and-responses.yml` - defines a set of responses to the flow's questions | ||
* `<smart-answer-flow-name>-responses-and-expected-results.yml` - a record of the question & outcome nodes visited when the above responses are applied combinatorially | ||
|
||
## Artefacts | ||
|
||
The following artefacts are saved in `test/artefacts`: | ||
|
||
* `<smart-answer-flow-name>/<smart-answer-flow-name>.txt` - rendered Govspeak for landing page | ||
* `<smart-answer-flow-name>/<responses-sequence>.html` - rendered HTML for question page | ||
* `<smart-answer-flow-name>/<responses-sequence>.txt` - rendered Govspeak for outcome pages | ||
|
||
The `<response-sequence>` is a forward-slash separated list of responses which closely relates to the URL paths to question & outcome pages in the app. | ||
|
||
The regression test fails if any of the following is true: | ||
|
||
* the newly generated artefact files differ at all from the committed files | ||
* not all nodes are exercised by the test data | ||
* the checksum data is out-of-date (see below) | ||
|
||
If you've added extra questions, responses or outcomes, then you should change the `test/data` files to exercise the new paths through the flow. See the instructions for [adding new regression tests](doc/adding-new-regression-tests.md) | ||
|
||
If there's a difference in the artefacts, you need to carefully review the changes to the artefacts to make sure they all relate to the changes you have made before committing them. | ||
|
||
Once you're happy that the changes to the files correspond to the changes to the test artefacts, you can update the checksums using the following command: | ||
|
||
$ rails r script/generate-checksums-for-smart-answer.rb <smart-answer-flow-name> | ||
|
||
When you've resolved all these issues, you should be able to run the regression tests for the flow as before and all the tests should pass. | ||
|
||
## Automatic trigger | ||
|
||
The regression test for a given flow are triggered automatically when you run the rake `test` task if you've made changed to any of the files whose checksums are listed in `test/data/<smart-answer-flow-name>-files.yml`. Although this won't always trigger the regression test when it should be run, it covers most common scenarios. | ||
|
||
If you've added new classes, modules or data which is used by a flow, you should add the relevant files to the checksums file. | ||
|
||
## Continuous integration | ||
|
||
The [main CI instance](https://ci-new.alphagov.co.uk/job/govuk_smart_answers/) and the [corresponding branches one](https://ci-new.alphagov.co.uk/job/govuk_smartanswers_branches/) run the rake `test` task and so work in the same way as above. | ||
|
||
We also have a [separate CI instance](https://ci-new.alphagov.co.uk/job/govuk_smart_answers_regressions/) which runs **all** the regression tests every so often. This should catch any scenarios missed by the automatic trigger. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly related to this PR, but I've skimmed the codebase (using GH search) and it looks like Phraselists are nearly extinct. I think it would make sense to look at removing it as a feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Yes, I think you're right that they have (almost?) entirely disappeared from question blocks. We already have a Trello card to remove documentation associated with them: https://trello.com/c/zODxvA3Y/78-remove-documentation-references-to-phraselists