Project

General

Profile

Actions

Bug #4227

closed

[Workbench] Pipeline elapsed time is misformatted

Added by Bryan Cosca over 9 years ago. Updated over 9 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
Workbench
Target version:
Story points:
0.5

Description

In pipeline_instances/qr1hi-d1hrv-43y08obrh2vetuv, it says This pipeline started at 11:56 AM 10/16/2014. It failed after 4 minutes 52 seconds at 12:01 AM 10/16/2014.

I expected it to say 12:01 "PM"


Subtasks 1 (0 open1 closed)

Task #4277: Review branch: 4227-date-displayResolvedRadhika Chippada10/21/2014Actions
Actions #1

Updated by Brett Smith over 9 years ago

  • Subject changed from Time log is wrong to [Workbench] Pipeline elapsed time is misformatted
  • Category set to Workbench
Actions #2

Updated by Radhika Chippada over 9 years ago

  • Status changed from New to In Progress
  • Assigned To set to Radhika Chippada
  • Target version set to 2014-10-29 sprint

The issue is in dates.js, another one of the under-tested javascript areas. Fix issue and add tests.

Actions #3

Updated by Ward Vandewege over 9 years ago

  • Story points set to 0.5
Actions #4

Updated by Brett Smith over 9 years ago

Reviewing e102efba. The fix is good, but I'm surprised at how involved the test code is.

  • I'm not sure I understand what value we get out of testing ten cases of this. It seems like we get the most value just by testing a simple case, a case that spans the AM/PM boundary, and a case that spans the midnight boundary. If there are other cases that have some unique property, then let's definitely include those. But ten seems like an arbitrary number, and that's a lot of overhead given that we're firing up a whole browser session to test datetime formatting.
    • I now limited to two tests, one with 0 run time and one with run time that spans between AM and PM
  • You can use DateTime::strptime to parse these strings, rather than doing all the splitting and array indexing yourself.
    • Thanks for this tip. It is fantastic
  • Rather than trying to parse the "elapsed time" string, let's use fixtures whose elapsed time we know ahead of time, and compare against that. That will save us a bunch of parsing and math that aren't directly relevant to what we're trying to test (the datetime formatting).
    • Adjusted this

BTW, there's also trailing whitespace on some of the lines in the current branch.

Thanks.

Actions #5

Updated by Brett Smith over 9 years ago

Thanks for following through on this. 99ad159 is good to merge, thanks.

Actions #6

Updated by Radhika Chippada over 9 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:6eaaae29a7af005e417673d79e0951122065e685.

Actions

Also available in: Atom PDF