grains: Update tests to match canonical-data

[denislooby]

Resolves #996

Removes multiple total_after tests in favor of one at the end as specified in canonical-data.

[cmccandless]

@denislooby Thanks for you pull request!

I'm not sure that removing the extra total_after calls is fully necessary. In particular, I think the calls inside the exception tests should probably remain, since there is no guarantee that total_after calls square (therefore invoking the same validation tests).

@N-Parsons thoughts?

[denislooby]

Yes, I was wondering about that.
The canonical-data and the test seems to have had different interpretations of the readme.

Write code that shows:
- how many grains were on each square, and
- the total number of grains

But you could argue for both ways.

[denislooby]

Should there be a 1-1 mapping between test asserts and cases in the canonical-data or is it less rigid than that?

[cmccandless]

@denislooby There's nothing wrong with multiple assertions in the same test case (for example: kindergarten-garden does this).

[denislooby]

@cmccandless I know multiple asserts in a test are ok, but should the canonical-data reflect all of them?
In kindergarten-garden it does seem to have them all in the json.

[cmccandless]

@denislooby as long as the additional asserts are neither redundant nor contradictory to the canonical ones, I would say that they are fine.

[denislooby]

@cmccandless Ok good to know. In that case definitely the exception test cases should be added back.
I'd be happy to add back the others too if you fell it would be for the best.

[cmccandless]

@denislooby I think simply restoring the exception test cases should be enough.

[N-Parsons]

@denislooby, thanks for your work on this.

(This comment got long, so TL;DR: I'm happy to merge once you've added the exception tests back)

I think that at the very least we should keep the exception tests for total_after - lots of implementations won't have the two functions connected, since there are mathematical tricks to jump straight to the answer, so testing for exception handling in both functions will be important.

Adding back the other tests would mean that the functions are being tested more, which would normally be considered a good thing, but realistically, testing one valid value and 3 exceptional values should be sufficient to fully test implementations of total_after. (In other languages these may be important/useful, but Python has no integer overflow). The aim of total_after with regard to the origin story is to know how many grains of rice are owed in total, so just testing square 64 seems fine to me.

[N-Parsons]

See comment above.

[denislooby]

@N-Parsons Changes done

[N-Parsons]

Thanks @denislooby! Sorry it took a while for me to get back to this.