Skip to content

Add a slightly longer sleep to a test that waits for a timer to advance#7466

Merged
jmhooper merged 1 commit intomainfrom
jmhooper-sleep-longer
Dec 9, 2022
Merged

Add a slightly longer sleep to a test that waits for a timer to advance#7466
jmhooper merged 1 commit intomainfrom
jmhooper-sleep-longer

Conversation

@jmhooper
Copy link
Contributor

@jmhooper jmhooper commented Dec 9, 2022

The sign in spec changed in this commit waits for a timer to advance. It waits exactly 1 second, which near the boundaries of the current second can lead to an edge case that leads to the following failure:

Failures:
  1) Sign in session approaches timeout user sees warning before session times out
     Failure/Error: expect(time2).to be < time1
       expected: < "14 minutes and 56 seconds"
            got:   "14 minutes and 56 seconds"
     # ./spec/features/users/sign_in_spec.rb:274:in `block (3 levels) in <top (required)>'
     # ./spec/rails_helper.rb:134:in `block (2 levels) in <top (required)>'

This commit modifies the sleep in this test to advance more than a second so that the assertion never runs near the boundary of the recent second.

The sign in spec changed in this commit waits for a timer to advance. It waits exactly 1 second, which near the boundaries of the current second can lead to an edge case that leads to the following failure:

```
Failures:
  1) Sign in session approaches timeout user sees warning before session times out
     Failure/Error: expect(time2).to be < time1
       expected: < "14 minutes and 56 seconds"
            got:   "14 minutes and 56 seconds"
     # ./spec/features/users/sign_in_spec.rb:274:in `block (3 levels) in <top (required)>'
     # ./spec/rails_helper.rb:134:in `block (2 levels) in <top (required)>'
```

This commit modifies the sleep in this test to advance more than a second so that the assertion never runs near the boundary of the recent second.

[skip changelog]
Copy link
Contributor

@jskinne3 jskinne3 left a comment

Choose a reason for hiding this comment

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

Best 2-byte commit ever

@aduth
Copy link
Contributor

aduth commented Dec 9, 2022

I don't suppose there's any way to avoid a real-time sleep and use something like Rails simulated time travel? Would be nice to avoid additional delays in the specs.

@jmhooper
Copy link
Contributor Author

jmhooper commented Dec 9, 2022

I agree, but there isn't anything obvious since this is waiting on the timer in the JS running in chromedriver. This commit fixes the flaky test and trades a half second for not having to wait for an entire 1/11 of the suite re-run.

@jmhooper jmhooper merged commit ca1914f into main Dec 9, 2022
@jmhooper jmhooper deleted the jmhooper-sleep-longer branch December 9, 2022 20:06
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.

3 participants