This PR adds some extra period validation, removes some duplicate validation and refactors plenty of tests.
Period envelopment validation
Some periods of time belong to other periods. For example, a MentorshipPeriod exists between ECTAtSchoolPeriod and MentorAtSchoolPeriod - it shouldn't stretch beyond any point in time that isn't covered by both the ECTAtSchoolPeriod and MentorAtSchoolPeriod.
The same 'containment' principle applies elsewhere too:
an InductionPeriod must be enveloped by its corresponding ECTAtSchoolPeriod
a TrainingPeriod for mentor training must be enveloped by its corresponding MentorAtSchoolPeriod
a TrainingPeriod for ECT training must be enveloped by its corresponding ECTAtSchoolPeriod
Making this change made lots of tests fail, which is why the refactoring is part of this PR.
Refactoring
The tests that broke because of the new validation were mainly because the factories don't pay attention to the scope of the 'parent' periods. We shouldn't really rely on factories to build the world so there's a little more setup. It might make sense to reintroduce something like ECF1's scenarios to handle these better.
To remove some duplication there is a list of common test cases in PeriodHelpers::PeriodExamples. They ensure that periods can:
not overlap their siblings
finish on the day their sibling starts (so there's no gap between periods)
Removing unique time boundaries
The checks that ensure two periods that belong to the same thing can't start/finish at the same time have been removed as those checks are handled by the overlap tests already.
Review suggestions
Probably worth going through commit by commit, paying most attention to 8623e8028bb70953f977c8e0534f6da39a78fb92
This PR adds some extra period validation, removes some duplicate validation and refactors plenty of tests.
Period envelopment validation
Some periods of time belong to other periods. For example, a
MentorshipPeriod
exists betweenECTAtSchoolPeriod
andMentorAtSchoolPeriod
- it shouldn't stretch beyond any point in time that isn't covered by both theECTAtSchoolPeriod
andMentorAtSchoolPeriod
.The same 'containment' principle applies elsewhere too:
InductionPeriod
must be enveloped by its correspondingECTAtSchoolPeriod
TrainingPeriod
for mentor training must be enveloped by its correspondingMentorAtSchoolPeriod
TrainingPeriod
for ECT training must be enveloped by its correspondingECTAtSchoolPeriod
Making this change made lots of tests fail, which is why the refactoring is part of this PR.
Refactoring
The tests that broke because of the new validation were mainly because the factories don't pay attention to the scope of the 'parent' periods. We shouldn't really rely on factories to build the world so there's a little more setup. It might make sense to reintroduce something like ECF1's scenarios to handle these better.
To remove some duplication there is a list of common test cases in
PeriodHelpers::PeriodExamples
. They ensure that periods can:Removing unique time boundaries
The checks that ensure two periods that belong to the same thing can't start/finish at the same time have been removed as those checks are handled by the overlap tests already.
Review suggestions
Probably worth going through commit by commit, paying most attention to 8623e8028bb70953f977c8e0534f6da39a78fb92