Display consistent format for job runtime

[kxk302]

This fixes the formatting reported in #10473

[kxk302]

@jmchilton or @dannon
Could you please take a look at this PR? Seems the issue can be fixed in backend only.
Thanks

[jdavcs]

Can we also have a unit test for seconds_to_str()? Also, maybe handle singular/plural; and also days? For example:

assert '1 second' == seconds_to_str(1)
assert '2 seconds' == seconds_to_str(2)
assert '1 minute' == seconds_to_str(60)
assert '1 minute 1 second' == seconds_to_str(61)
assert '1 minute 2 seconds' == seconds_to_str(62)
assert '2 minutes 1 second' == seconds_to_str(121)
etc...

If this is too annoying to handle, I think there might be a library for this.

[dannon]

@kxk302 This looks great to me -- that was definitely a bug before, good catch. Do you want to rebase this branch to drop the other commits related to the vscode config?

[kxk302]

Can we also have a unit test for seconds_to_str()? Also, maybe handle singular/plural; and also days? For example:

assert '1 second' == seconds_to_str(1)
assert '2 seconds' == seconds_to_str(2)
assert '1 minute' == seconds_to_str(60)
assert '1 minute 1 second' == seconds_to_str(61)
assert '1 minute 2 seconds' == seconds_to_str(62)
assert '2 minutes 1 second' == seconds_to_str(121)
etc...

If this is too annoying to handle, I think there might be a library for this.

I can add unit tests for sure, probably tomorrow though.

[kxk302]

@kxk302 This looks great to me -- that was definitely a bug before, good catch. Do you want to rebase this branch to drop the other commits related to the vscode config?

I can cherry-pick this change into my other branch, so we'd have 1 PR. Would that work?

[dannon]

@kxk302 I think it's fine to leave this as a separate PR since it's totally unrelated. I'd just do a simple git rebase -i upstream/dev, drop the preceding 4 commits from the rebase plan, and it should be good to go. Then you force-push over this branch if it looks good, and the PR will auto-update.

[kxk302]

@kxk302 I think it's fine to leave this as a separate PR since it's totally unrelated. I'd just do a simple git rebase -i upstream/dev, drop the preceding 4 commits from the rebase plan, and it should be good to go. Then you force-push over this branch if it looks good, and the PR will auto-update.

Got it. Sorry I become slower near the end of the day :)

[kxk302]

@dannon Updated string formatting

[dannon]

@kxk302 Thanks. Next time you can just 'accept suggestions' (in batch if there are multiple), too, and it'll make a commit for you, also crediting the suggester.

[kxk302]

Added unit tests.

[kxk302]

The only check failing seems to client test (ToolsView.test.js). I don't think this is related to my code change. Could someone please verify and if so merge this? Thanks.

[jdavcs]

@kxk302 thank you for adding the test! See my inline comment. Plural vs. singular - up to you. I'm OK either way.

[kxk302]

@kxk302 thank you for adding the test! See my inline comment. Plural vs. singular - up to you. I'm OK either way.

@ic4f I updated the method and the test to handle singular/plural seconds/minutes. Made the method a bit convoluted, but I guess nobody can pick on the grammar :)

[jdavcs]

Great! As a suggestion, you could simplify this, if you like. It's just 3 cases: value < 60; if not then value < 3600; and the rest - i.e., if/elif/else. As for the s suffix - you could pre-calculate it: it's '' or 's' for secs/mins/hours depending on whether they == 1.

[jdavcs]

Thank you for making all the changes, looks great!