76 irvcomparisonaudit #130
Conversation
…re necessary, and updated the Assertion tests. Still need extra tests for new Assertion functionality and the functionality in IRVComparisonAudit.
…e-auditing and the fact that the removal/recording methods are designed to be called multiple times for a specific CVRAuditInfo (if the CVR appears multiple times in the sample). Adapted commenting to make this clear.
…ting ballots. Added missed check of excess removal of two vote overstatements from assertion totals.
…s of discrepancies will result in an exception.
…tabase. Changed name of assumed_context in assertion to assertion_assumed_continuing. Added some initial tests of IRVComparisonAudit construction.
Added annotations to BigDecimals in Assertion to have the same precision and scale as those in ComparisonAudit. Updated corla.sql in the test resources with the new database schema.
| @Override | ||
| protected void recalculateSamplesToAudit() { | ||
| if(assertions == null){ | ||
| // We have not associated assertions with this audit yet. When an IRVComparisonAudit | ||
| // constructed, we call the base class constructor first. The ComparisonAudit constructor |
| @Override | ||
| public int initialSamplesToAudit() { | ||
| return 0; |
There was a problem hiding this comment.
This is indeed never used, and the comment in the base class is pretty brief, but my reading of it is that you're meant to be able to call it at any time, and get the initial sample estimate (when there have been no errors). I don't think this is quite what your method would do if there already were errors - I think it'd return the same thing as getOptimisticSamplesToAudit().
If we didn't override it, we'd just get computeOptimisticSamplesToAudit(0,0,0,0) (i.e. with zero discrepancies everywhere) - it's not clear to me that that's not fine, but it's possible I'm confused.
There was a problem hiding this comment.
Added method to Assertion class to compute initial samples to audit and then called this in the IRVComparisonAudit method.
| // and the flag for indicating that a sample size recalculation is needed, *if* we did | ||
| // record a discrepancy against at least one of this audit's assertions. | ||
| if(recorded) { | ||
| super.removeDiscrepancy(theRecord, theType); |
There was a problem hiding this comment.
Is this meant to be 'remove'? I think I would have expected it to be 'record', but I could be confused.
| * Test the re-auditing of a ballot where a prior discrepancy is recorded against the | ||
| * associated CVR (a two vote overstatement). The existing discrepancies associated with the | ||
| * 'n' copies of the CVR in the sample are removed, but removeDiscrepancy() is called n+1 times | ||
| * in error. The n+1'th call the removeDiscrepancy should throw an exception. |
| @@ -238,7 +241,7 @@ public void testNENEstimatedVaryingSamples(Integer auditedSamples, BigDecimal ri | |||
| * there, the value matching the ID key is retrieved and the associated discrepancy type | |||
| * incremented. If it is not there, then the discrepancy counts are not changed. | |||
| */ | |||
| @Test(expectedExceptions = {RuntimeException.class}) | |||
| @Test | |||
| public void testNENRecordNoMatch1(){ | |||
There was a problem hiding this comment.
These could be refactored into one big parameterized test.
There was a problem hiding this comment.
Same comment for NEBAssertionsTests.
There was a problem hiding this comment.
It would be a bit of a pain to do this, particularly as the expected outcomes/contexts are different enough for each case. However, there was an opportunity for abstracting out some common code across a subset of these record/remove tests (ie. checking of discrepancy counts).
…/NEB Assertion Tests.
…time during audit.
There is only a couple of IRVComparisonAudit tests currently that looks at initialisation. More will be added in a later card.