Bug #5276

[Workbench] Log graph pop up shows when there is no log graph

Added by Bryan Cosca almost 6 years ago. Updated over 5 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench
Target version:
Start date:
02/18/2015
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
0.5

Description

log_graph_popup_shows.gif (224 KB) log_graph_popup_shows.gif Bryan Cosca, 02/18/2015 10:51 PM

Subtasks

Task #5489: Reproduce or find plausible explanationResolvedTom Clegg

Task #5516: Review 5276-job-graph-phantom-tooltipResolvedTom Clegg


Related issues

Related to Arvados - Bug #5474: [Workbench] WebsocketTest#test_live_log_charting is not reliableResolved03/16/2015

Associated revisions

Revision 6381b9e8
Added by Tom Clegg over 5 years ago

Merge branch '5276-job-graph-phantom-tooltip' closes #5276

History

#1 Updated by Brett Smith almost 6 years ago

  • Subject changed from The log graph pop up shows when there is no log graph to [Workbench] Log graph pop up shows when there is no log graph
  • Category set to Workbench
  • Target version set to Bug Triage

#2 Updated by Tom Clegg over 5 years ago

  • Assigned To set to Tom Clegg
  • Target version changed from Bug Triage to 2015-04-01 sprint

#3 Updated by Tom Clegg over 5 years ago

  • Story points set to 0.5

#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.)

#6 Updated by Brett Smith over 5 years ago

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.

#7 Updated by Anonymous over 5 years ago

  • Status changed from New to Resolved
  • % Done changed from 50 to 100

Applied in changeset arvados|commit:6381b9e85f278b0c8cb45ffccd89ca1b1bc4d3ee.

Also available in: Atom PDF