Arvados

Reviewing ea344b06, and I just have one pithy test style comment. Where you test against page.evaluate_script to avoid Capybara's timeout, I think it should be possible to use using_wait_time(0) do … end to run an assertion without any retrying. I think that might be a little more readable and self-documenting. But if I'm wrong or you prefer page.evaluate_script for other reasons, this is fine to merge. Thanks.

Brett Smith wrote:

I think it should be possible to use using_wait_time(0) do … end

Good point. Fixed, a6ea501.

Also updated the JS changes, after checking with Bryan that the bug has only happened in the first 5 seconds (it didn't) and running a few more test jobs to see if I could still reproduce it with this change (I could).

The other improvements are good too, but are now accompanied by:

• Wait until show() finishes before creating the graph. Otherwise Morris creates an SVG element 1px high, which leaves hoverable tooltip triggers in the expected positions but puts the graph itself outside its containing div and therefore invisible. (I think this was the real bug all along; the 5-second delay just made it more confusing to watch/debug.)

• Never show a tooltip on the placeholder data point. (This is pretty much unrelated, but it was an easy when I noticed it was happening.)

Tom Clegg wrote:

Brett Smith wrote:

I think it should be possible to use using_wait_time(0) do … end

Good point. Fixed, a6ea501.

This is good to merge. Thanks.