Navigation Menu

Skip to content
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 regression test to ensure all nodes are reached #1668

Merged
merged 10 commits into from Jun 1, 2015

Conversation

chrisroos
Copy link
Contributor

By storing both the current_node and next_node in the responses-and-expected-results YAML file, we can compare the nodes that have been exercised with all the nodes in the Flow. I'd added a test that fails if the two sets of nodes don't match.

This is the test that @floehopper mentioned in 53e67d8.

chrisroos referenced this pull request May 28, 2015
These response combinations give full test coverage of the flow & calculator
code, except for:

* Validation for `money_question :how_much_extra_per_week?`. However, it not
currently possible to test error states and in any case doing so is not
relevant to introducing ERB outcome templates which is the current aim of
introducing these regression tests.

* The `total = 0` line in the `else` condition in
`StatePensionTopupCalculator#lump_sum_amount`. However, this code appears to be
unreachable via the user interface.

Test coverage alone wasn't sufficient to generate all the possible outcomes, so
I had to generate a suitable range of responses by inspecting the code.

@chrisroos plans to introduce a check that all outcomes have been generated, so
if I've missed any that should be caught then.
@floehopper
Copy link
Contributor

Did all the existing regression tests just pass once you'd regenerated their responses-and-expected-results? I assume they did.

LGTM 👍

Having the current and next nodes will allow me to add a test that fails unless
all nodes from the flow are being exercised.
This test collects all of the current and next nodes from the
responses-and-expected-results file and ensures that the unique set of them
matches `Flow#nodes`. This should help us catch the case where our
questions-and-responses file doesn't contain enough data to exercise every node.
Regenerated using:

    $ rails r script/generate-responses-and-expected-results-for-smart-answer.rb \
      additional-commodity-code

I also had to update the file checksums, using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
      additional-commodity-code
Regenerated using:

    $ rails r script/generate-responses-and-expected-results-for-smart-answer.rb \
      am-i-getting-minimum-wage

I also had to update the file checksums, using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
      am-i-getting-minimum-wage
Regenerated using:

    $ rails r script/generate-responses-and-expected-results-for-smart-answer.rb \
      apply-tier-4-visa

I also had to update the file checksums, using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
      apply-tier-4-visa
Regenerated using:

    $ rails r script/generate-responses-and-expected-results-for-smart-answer.rb \
      state-pension-through-partner

I also had to update the file checksums, using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
      state-pension-through-partner
Regenerated using:

    $ rails r script/generate-responses-and-expected-results-for-smart-answer.rb \
      state-pension-topup

I also had to update the file checksums, using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
      state-pension-topup
Regenerated using:

    $ rails r script/generate-responses-and-expected-results-for-smart-answer.rb \
      student-finance-calculator

I also had to update the file checksums, using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
      student-finance-calculator
Regenerated using:

    $ rails r script/generate-responses-and-expected-results-for-smart-answer.rb \
      towing-rules

I also had to update the file checksums, using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
      towing-rules
Regenerated using:

    $ rails r script/generate-responses-and-expected-results-for-smart-answer.rb \
      vat-payment-deadlines

I also had to update the file checksums, using:

    $ rails r script/generate-checksums-for-smart-answer.rb \
      vat-payment-deadlines
@chrisroos chrisroos force-pushed the add-regression-test-to-ensure-all-nodes-are-reached branch from 74e433d to bed30a1 Compare June 1, 2015 13:13
@chrisroos
Copy link
Contributor Author

Did all the existing regression tests just pass once you'd regenerated their responses-and-expected-results? I assume they did.

They did indeed!

@chrisroos
Copy link
Contributor Author

I've updated this branch to include the updated responses-and-expected-outputs and checksum data for the regression tests that have been added since last week. I'm going to get this merged to master.

@chrisroos chrisroos merged commit bed30a1 into master Jun 1, 2015
@chrisroos chrisroos deleted the add-regression-test-to-ensure-all-nodes-are-reached branch June 1, 2015 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants