[#75] Fix a timing bug with poltergeist/turbolinks #76
Conversation
There's a timing bug in ViewHelperTest that randomly results in ```ReferenceError: Turbolinks Not Found```. This causes unrelated builds to fail, which gums up the whole open source works. Per https://github.com/teampoltergeist/poltergeist#timing-problems, the suggested way of fixing this is to just add calls to sleep. If you wish to test this in the future, use the shell code: ```for i in `seq 1 10`; do rake appraisal; done``` This should give you enough test runs to hit the timing bug at least once.
|
I have been working with some more sophisticated scenarios with Capybara lately - I think depending on the version either |
|
@JakubMal I'd love to see your scenarios, I'm not up-to-date with Capybara and would definitely appreciate looking at something more sophisticated. And I totally agree this needs improvement. I tried, but I was unable to get # wait_until was removed from Capybara 2
require 'timeout'
def wait_until
Timeout.timeout(Capybara.default_wait_time) do
until value = yield
sleep 0.1
end
end
value
end
# ...snip...
wait_until { page.evaluate_javascript 'typeof Turbolinks !== "undefined"' }
page.execute_script('Turbolinks.visit("/pages/2");')At most, it would sleep one cycle, but still throw a ReferenceError. After spending about 2 hours trying to get that to work, I gave up and put in FWIW, I haven't been able to find anyone else on Github that is testing Turbolinks by calling |
|
Because you need to |
The code in the block wouldn't raise an error - it's just evaluating the response. That's what's weird about it -
I didn't write the test, I was only trying to fix it. I think the original intent was to assert that the Turbolinks library itself would keep working with ReactJS. The test probably needs a refactor. |
|
Sorry I had a certain preconception of how this should be solved, and assumed you were thinking about it the same way. There is a couple of problems with your solution: Which version of Capybara supports However - I cannot reproduce the problem with: I hope it is self-explaining. Then this: with this code in test_helper: Would you mind trying to reproduce using the above code? (without my faking the Turbolinks not being loaded) One more thing - I think you can see that this snippet doesn't test anything:
If you confirm that you cannot reproduce the problem with my code - I'll follow with a PR. |
|
I'm totally fine with ripping the direct calls to Turbolinks.visit out of the test suite. I'm pretty sure that nobody in the world is actually testing Turbolinks this way, and I'm not surprised that it's flakey. If you search Github for "test Turbolinks.visit", we are the only Ruby repository that actually is testing it like this. For example, Steve Klabnik's performance test of Turbolinks just uses
That's really strange! Did you try more than 10 runs? For me, it's not guaranteed that I'll get the error, but it usually shows up 1-in-10 times. Note that a failure doesn't halt execution of the test suite, and since it takes so long to run if you aren't watching it you'll have to scroll.
I'm very sorry, I was typing from memory, not copying working code, and it looks like my example included a typo. The correct call is
That's why I was testing to see if Turbolinks was undefined. But then I was still getting errors, so I gave up and used sleep.
I threw some puts in your code block, and it's definitely working in some cases: However, I am somewhat reliably seeing this: With a 10x run, I saw this error 4 times. I'm running this on a 15" Retina Macbook Pro, 2.7ghz i7. |
|
An example alternative for intentionally testing Turbolinks vs No Turbolinks: Spec: https://github.com/steveklabnik/turbolinks_test/blob/master/spec/requests/turbolinks_spec.rb |
|
It makes sense to move to What's your Phantomjs version? Does this error happen only for rails < 4.0 ? |
1.9.7
That would be interesting, but no 😢. I've seen it at least with = 4.0: |
There's a timing bug in ViewHelperTest that randomly results in
ReferenceError: Turbolinks Not Found. This causes unrelated buildsto fail, which gums up the whole open source works.
Per https://github.com/teampoltergeist/poltergeist#timing-problems, the
suggested way of fixing this is to just add calls to sleep.
If you wish to test this in the future, use the shell code:
for i inseq 1 10; do rake appraisal; doneThis should give you enough test runs to hit the timing bug at least
once.