Mochitest for NetworkStats failure

RESOLVED FIXED in mozilla24

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: albert, Assigned: albert)

Tracking

unspecified
mozilla24
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fixed-in-birch])

Attachments

(1 attachment)

(Assignee)

Description

5 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
(Assignee)

Updated

5 years ago
Assignee: nobody → acperez
(Assignee)

Comment 1

5 years ago
Created attachment 753189 [details] [diff] [review]
Fix some failures in networkstats when running mochitest
Attachment #753189 - Flags: review?(gene.lian)
(Assignee)

Updated

5 years ago
Blocks: 858005
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?
(Assignee)

Comment 3

5 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.
(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!
Attachment #753189 - Flags: review?(gene.lian) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/projects/birch/rev/a5c6f83911dd
Keywords: checkin-needed
Whiteboard: [fixed-in-birch]
https://hg.mozilla.org/mozilla-central/rev/a5c6f83911dd
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.