Skip to content
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.

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

Siteprism hangs when used with timecop #388

Closed
dkniffin opened this issue Apr 25, 2019 · 3 comments
Closed

Siteprism hangs when used with timecop #388

dkniffin opened this issue Apr 25, 2019 · 3 comments

Comments

@dkniffin
Copy link
Contributor

I've just discovered an issue with interactions between site_prism and timecop. It looks like this is a known issue (#88), which was closed (completely reasonably).

To describe the issue again here: If you are using timecop with Timecop.freeze somewhere in your code (either in the feature/system test, or in another test with leakage), Waiter.wait_until_true will hang because Time.now does not change.

This actually isn't a bug with Site Prism, but I just spent a few hours tracking this down, so I wanted to create an issue, to hopefully avoid this issue for others in the future. At the very least, I'm hoping this issue will come up in a Google search, but I'm also wondering if there's some documentation or messaging we could add that might help as well.

My two thoughts:

  1. We could put a bit of code in somewhere that checks for the presence of Timecop and if it's there, just output some logging that says something like "fyi, you're using timecop"
  2. We could add a "known issues" section to the readme listing this, so people can find it.
@luke-hill
Copy link
Collaborator

I know previously we've had some Timeout.timeout calls we fixed for this, and I know Capybara have got an experimental PR (Thomas has had this for a while), to crash when people call Timecop.freeze

As mentioned in the original Issue request, browser integration tests should never have frozen time. Whereas maybe stubbing it in specs is fine (However we do have some time management unit tests also which may have issues with this)

Both of your points are completely valid, and I'm happy to listen to PR's for them. At the moment I'm deep inside a couple of gritty features (Which are on the repo and have been for a while), so I'd prefer to keep my focus on tackling these.

https://github.com/teamcapybara/capybara/pull/2032/files - This is the PR which is being experimented with to prevent time freezes.

Caveats to my acceptance of your above ideas are...

  1. What should we do if we encounter frozen time..
  • Crash
  • Logger messages (site_prism has it's own logging API)
  1. Should we add timecop as a dependency and monitor it, or perhaps (Not sure if this is possible), ban the usage of timecop with site_prism

I'll also happily listen to a listener-poll inside the wait_until_true method that after each tick checks whether the elapsed time was >0 (And if it wasn't throw an error and exit with a FrozenInTimeError

Hopefully that's given you a few nuggets of wisdom to debate. But at the moment I wouldn't be able to commit to coding it. If you or others watching are then I'm all for extra functionality / guards.

@dkniffin
Copy link
Contributor Author

Thanks for the quick response! I like the idea of checking in the wait_until_true and throwing a FrozenInTimeError. I'll open a PR for that.

Also, somewhat related, I thought about this a little and realized it's really a bigger issue and could be addressed with a rubocop cop. I did some searching and found this: rubocop/rubocop-rails#38

@luke-hill
Copy link
Collaborator

Closed by b68171a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants