Albert [:albert] Assignee
	Description • 11 years ago

Mochitest for networkstats are failing:

 python runtestsb2g.py --desktop --console-level INFO --profile B2G/gaia/profile --test-path dom/network
62 INFO TEST-START | Shutdown
63 INFO Passed: 45
64 INFO Failed: 8
65 INFO Todo:   0
66 INFO SimpleTest FINISHED
67 INFO TEST-INFO | Ran 0 Loops
68 INFO SimpleTest FINISHED

	Gene Lian (I already quit Mozilla)
	Comment 2 • 11 years ago

Comment on attachment 753189 [details] [diff] [review]
Fix some failures in networkstats when running mochitest

Review of attachment 753189 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/network/src/NetworkStatsDB.jsm
@@ +269,5 @@
>          this.fillResultSamples(start, end, data);
>  
>          txn.result.connectionType = aOptions.connectionType;
> +        txn.result.start = aOptions.start;
> +        txn.result.end = aOptions.end;

OK. I'm assuming you're hoping to return the start/end as results with Date types based on the input instead of the UTC-adjusted value.

Anyway, this is not the root cause because |new Date(new Date)| still works in my JS console. Right?

::: dom/network/tests/test_networkstats_basics.html
@@ +51,5 @@
>  }
>  
>  function checkDataDates(data, start, end, sampleRate){
> +  start = Math.floor(start.getTime() / sampleRate) * sampleRate;
> +  end = Math.floor(end.getTime() / sampleRate) * sampleRate;

Sorry I don't quite follow the fix here. Could you briefly explain about what's the difference with and without the timezone offset?

@@ +153,1 @@
>      var startDate = new Date(endDate.getTime() - (sampleRate * diff));

Why didn't you do the same thing for startDate in the original codes? And why the new patch fixes the test after removing offset?

	Albert [:albert] Assignee
	Comment 3 • 11 years ago

(In reply to Gene Lian [:gene] from comment #2)
> Comment on attachment 753189 [details] [diff] [review]
> Fix some failures in networkstats when running mochitest
> 
> Review of attachment 753189 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/network/src/NetworkStatsDB.jsm
> @@ +269,5 @@
> >          this.fillResultSamples(start, end, data);
> >  
> >          txn.result.connectionType = aOptions.connectionType;
> > +        txn.result.start = aOptions.start;
> > +        txn.result.end = aOptions.end;
> 
> OK. I'm assuming you're hoping to return the start/end as results with Date
> types based on the input instead of the UTC-adjusted value.
> 
> Anyway, this is not the root cause because |new Date(new Date)| still works
> in my JS console. Right?

aOptions.start and aOptions.end are Date objects so there is no need to create the Date again. The problem is that creating the Date of a Date is wrong in firefox, it remove millis precision and it causes a missmatch in mochitest. Try the following in your JS console:

a= new Date();
a.getTime();
(new Date(a)).getTime();

> ::: dom/network/tests/test_networkstats_basics.html
> @@ +51,5 @@
> >  }
> >  
> >  function checkDataDates(data, start, end, sampleRate){
> > +  start = Math.floor(start.getTime() / sampleRate) * sampleRate;
> > +  end = Math.floor(end.getTime() / sampleRate) * sampleRate;
> 
> Sorry I don't quite follow the fix here. Could you briefly explain about
> what's the difference with and without the timezone offset?
>
> @@ +153,1 @@
> >      var startDate = new Date(endDate.getTime() - (sampleRate * diff));
> 
> Why didn't you do the same thing for startDate in the original codes? And
> why the new patch fixes the test after removing offset?


In another bug the way to manage timezones was modified and now the offset is processed internally by the API to avoid problems when the clock time is changed by the user, so here is not necessary to take it into account.

	Gene Lian (I already quit Mozilla)
	Comment 4 • 11 years ago

(In reply to Albert from comment #3)
> (In reply to Gene Lian [:gene] from comment #2)
> The problem is that creating the Date of a Date is
> wrong in firefox, it remove millis precision and it causes a missmatch in
> mochitest. Try the following in your JS console:
> 
> a= new Date();
> a.getTime();
> (new Date(a)).getTime();

This really sounds bad... I'll fire a bug later so see if someone can take this. Nice catch!

	Gene Lian (I already quit Mozilla)
	Comment 5 • 11 years ago

Fire Bug 875668.

	Gene Lian (I already quit Mozilla)
	Comment 6 • 11 years ago

https://hg.mozilla.org/projects/birch/rev/a5c6f83911dd

	Ed Morley [:emorley]
	Comment 7 • 11 years ago

https://hg.mozilla.org/mozilla-central/rev/a5c6f83911dd