[Workbench] Log graph pop up shows when there is no log graph
#4 Updated by Brett Smith over 5 years ago
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.
#5 Updated by Tom Clegg over 5 years ago
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.)