Skip to content

LG-11441 - Fix Flakey In-person Enrollment test #9477

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.

Sign up for GitHub

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

Merged
JackRyan1989 merged 5 commits into from
Nov 1, 2023
Merged

LG-11441 - Fix Flakey In-person Enrollment test #9477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
70b2269
changelog: Internal, In-Person Proofing, Fix intermittent test failure
Oct 28, 2023
fa67fbb
Adding specs to confirm date calculation for ipp enrollment
Oct 30, 2023
b0fcb18
adding let blocks to clarify dates
Oct 30, 2023
fcc325b
duration checks use configuration values now
Oct 31, 2023
6e3154f
Updating spec to use validity in days from config
Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/models/in_person_enrollment.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,8 @@ def due_date
end

def days_to_due_date
today = DateTime.now
(today...due_date).count
today = Time.zone.now.utc
(due_date - today).seconds.in_days.to_i
end

def eligible_for_notification?
Expand Down
50 changes: 45 additions & 5 deletions spec/models/in_person_enrollment_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,6 @@

describe 'due_date and days_to_due_date' do
let(:validity_in_days) { 10 }
let(:days_ago_established_at) { 7 }

before do
allow(IdentityConfig.store).
Expand All @@ -400,10 +399,10 @@
freeze_time do
enrollment = create(
:in_person_enrollment,
enrollment_established_at: days_ago_established_at.days.ago,
enrollment_established_at: (validity_in_days - 3).days.ago,
)
expect(enrollment.due_date).to(
eq((validity_in_days - days_ago_established_at).days.from_now),
eq(3.days.from_now),
)
end
end
Expand All @@ -412,9 +411,50 @@
freeze_time do
enrollment = create(
:in_person_enrollment,
enrollment_established_at: days_ago_established_at.days.ago,
enrollment_established_at: (validity_in_days - 3).days.ago,
)
expect(enrollment.days_to_due_date).to eq(validity_in_days - days_ago_established_at)
expect(enrollment.days_to_due_date).to eq(3)
end
end

context 'check edges to confirm date calculation is correct' do
it 'returns the correct due date and days to due date with 1 day left' do
freeze_time do
enrollment = create(
:in_person_enrollment,
enrollment_established_at: (validity_in_days - 1).days.ago,
)
expect(enrollment.days_to_due_date).to eq(1)
expect(enrollment.due_date).to(
eq(Time.zone.now + 1.day),
)
end
end

it 'returns the correct due date and days to due date with 0.5 days left' do
freeze_time do
enrollment = create(
:in_person_enrollment,
enrollment_established_at: (validity_in_days - 0.5).days.ago,
)
expect(enrollment.days_to_due_date).to eq(0)
Comment on lines +438 to +440
Copy link
Copy Markdown
Contributor

@aduth aduth Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just want to double-check that rounding down to zero is the expected result here? I see some related subtraction math on this number that might worry me if we tell the user that they "have -1 days remaining" or some such.

identity-idp/app/presenters/idv/in_person/ready_to_verify_presenter.rb

Lines 17 to 20 in 6a4d5db

# Reminder is exclusive of the day the email is sent (1 less than days_to_due_date)
def days_remaining
enrollment.days_to_due_date - 1
end

Copy link
Copy Markdown
Contributor Author

@JackRyan1989 JackRyan1989 Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aduth great callout. I think we're ok. I'm currently digging through the ready_to_verify_presenter_spec: I just added two tests locally for 1 day remaining and 0 days remaining. 1 day remaining works, 0 days remaining fails because we get a -1, as expected by your observation.

The other test already in that file passes, so I think the behavior is expected. We do have a in_person_deadline_passed email that goes out, and I'm just digging around now for the logic that controls when that is sent so I can confirm that on 0 days remaining that email is sent and the in_person_ready_to_verify_reminder email is not. I'll confirm in another message once I'm done digging.

Copy link
Copy Markdown
Contributor Author

@JackRyan1989 JackRyan1989 Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the email_reminder_job the latest we send the reminder is 1 day before expiration. In that case we'd say '0 days left' or something similar, and we would not send a negative value.

Copy link
Copy Markdown
Contributor

@aduth aduth Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok 👍 Thanks for checking

expect(enrollment.due_date).to(
eq(Time.zone.now + 0.5.days),
)
end
end

it 'returns the correct due date and days to due date with 0 days left' do
freeze_time do
enrollment = create(
:in_person_enrollment,
enrollment_established_at: validity_in_days.days.ago,
)
expect(enrollment.days_to_due_date).to eq(0)
expect(enrollment.due_date).to(
eq(Time.zone.now),
)
end
end
end
end
Expand Down