Add shouldBeNow() to js-tests.js in LayoutTests.

Add shouldBeNow() to js-tests.js in LayoutTests.

The function checks that an expression returns the current time. The
check is non-trivial, given that time might go backwards due to NTP
updates.

BUG=320686
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162645

[pwnall-personal, 2013-11-18 16:55:45]

The File API mandates that the lastModifiedTime property returns the current
time under some circumstances. I started writing the LayoutTest for that and
ended up with something too complex to be copied across tests, so I'd like to
make it generally available.

Can you please take a look?

[pwnall-personal, 2013-11-18 21:04:16]

On 2013/11/18 16:55:45, pwnall wrote:
> The File API mandates that the lastModifiedTime property returns the current
> time under some circumstances. I started writing the LayoutTest for that and
> ended up with something too complex to be copied across tests, so I'd like to
> make it generally available.
> 
> Can you please take a look?

I just learned that Date.now() is based off of UTC, so it isn't impacted by DST.
NTP updates can still skew the clock though, so I think that having a robust
test is still valuable for avoiding flakes.

[pwnall-personal, 2013-11-21 02:16:39]

On 2013/11/18 21:04:16, pwnall wrote:
> On 2013/11/18 16:55:45, pwnall wrote:
> > The File API mandates that the lastModifiedTime property returns the current
> > time under some circumstances. I started writing the LayoutTest for that and
> > ended up with something too complex to be copied across tests, so I'd like
to
> > make it generally available.
> > 
> > Can you please take a look?
> 
> I just learned that Date.now() is based off of UTC, so it isn't impacted by
DST.
> NTP updates can still skew the clock though, so I think that having a robust
> test is still valuable for avoiding flakes.

ping?

Can you please take a look at this?

I used a less robust check in a layout test and it caused flakiness on XP. I
would really like to have this method available to my tests.

Thank you very much!

[ojan, 2013-11-23 01:41:30]

I'm not opposed to this, but I'm having trouble seeing the value. Other than the
one test that's part of this patch, when else would we use this function?

https://codereview.chromium.org/69813015/diff/80001/LayoutTests/resources/js-...
File LayoutTests/resources/js-test.js (right):

https://codereview.chromium.org/69813015/diff/80001/LayoutTests/resources/js-...
LayoutTests/resources/js-test.js:574: for (var _i = 0; _i < 1000; ++_i) {
No need for the underscores. jsbell has a patch removing the other underscore
prefixes from this file.

[pwnall-personal, 2013-11-23 01:47:26]

On 2013/11/23 01:41:30, ojan wrote:
> I'm not opposed to this, but I'm having trouble seeing the value. Other than
the
> one test that's part of this patch, when else would we use this function?
> 
>
https://codereview.chromium.org/69813015/diff/80001/LayoutTests/resources/js-...
> File LayoutTests/resources/js-test.js (right):
> 
>
https://codereview.chromium.org/69813015/diff/80001/LayoutTests/resources/js-...
> LayoutTests/resources/js-test.js:574: for (var _i = 0; _i < 1000; ++_i) {
> No need for the underscores. jsbell has a patch removing the other underscore
> prefixes from this file.

I tried a simpler version of checking against Date.now() that basically did t0 =
Date.now(); t = testedCode(); t1 = Date.now() and then checked t0 <= t <= t1.

The simpler version passed on all trybots and a CL got committed, then it caused
flakiness in XP, and the CL got reverted. This caused work for the sheriff and
will cause extra work for reviewers :(

The File API has a "lastModified" attribute (used to be "lastModifiedDate") that
is supposed to return the current date/time when the underlying file's mtime
isn't known.

I'd like to use this right now in tests for the File constructor.
https://codereview.chromium.org/80983002/

I'm guessing the filesystem API might want this as well, since it deals with
File objects, but I haven't gone through that code yet.

What do you think? Does this sound reasonable?

[pwnall-personal, 2013-11-23 01:56:22]

https://codereview.chromium.org/69813015/diff/80001/LayoutTests/resources/js-...
File LayoutTests/resources/js-test.js (right):

https://codereview.chromium.org/69813015/diff/80001/LayoutTests/resources/js-...
LayoutTests/resources/js-test.js:574: for (var _i = 0; _i < 1000; ++_i) {
On 2013/11/23 01:41:31, ojan wrote:
> No need for the underscores. jsbell has a patch removing the other underscore
> prefixes from this file.

Done.

[ojan, 2013-11-25 22:50:31]

lgtm

https://codereview.chromium.org/69813015/diff/240001/LayoutTests/resources/js...
File LayoutTests/resources/js-test.js (right):

https://codereview.chromium.org/69813015/diff/240001/LayoutTests/resources/js...
LayoutTests/resources/js-test.js:574: for (var i = 0; i < 1000; ++i) {
We'll have to see how long this takes in practice on the bots and lower it if
it's close to the test timeout.

[commit-bot: I haz the power, 2013-11-25 22:50:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/costan@gmail.com/69813015/240001

[commit-bot: I haz the power, 2013-11-26 00:24:05]

Change committed as 162645