Closed Bug 1405243 Opened 2 years ago Closed 2 years ago

Migrate browser_webconsole_bug_1006027_message_timestamps_incorrect.js to a server test

Categories

(DevTools :: Console, enhancement, P1)

enhancement

Tracking

(firefox57 wontfix, firefox58 wontfix, firefox61 fixed)

RESOLVED FIXED
Firefox 61
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix
firefox61 --- fixed

People

(Reporter: nchevobbe, Assigned: nchevobbe)

References

(Blocks 1 open bug)

Details

(Whiteboard: [newconsole-mvp])

Attachments

(2 files)

This test should be on the server to make sure we always send the correct timestamp in console log packet
Priority: -- → P3
Assignee: nobody → abhinav.koppula
Status: NEW → ASSIGNED
Priority: P3 → P1
Whiteboard: [newconsole-mvp]
Assignee: abhinav.koppula → nobody
Status: ASSIGNED → NEW
Priority: P1 → P2
Assignee: nobody → nchevobbe
Status: NEW → ASSIGNED
Priority: P2 → P1
I'm seeing this error in mozreview: "You don't have access to this review request. This review request is private. You must be a requested reviewer, either directly or on a requested group, and have permission to access the repository in order to view this review request."
Flags: needinfo?(nchevobbe)
Comment on attachment 8960201 [details]
Bug 1405243 - Migrate browser_webconsole_bug_1006027_message_timestamps_incorrect.js to a server test; .

https://reviewboard.mozilla.org/r/228956/#review234696

::: commit-message-0b997:3
(Diff revision 1)
> +Bug 1405243 - Migrate browser_webconsole_bug_1006027_message_timestamps_incorrect.js to a server test; r=bgrins.
> +
> +The test was testing that the 3 messages we display whe

Typo "when"

::: devtools/shared/webconsole/test/test_console_timestamp.html:32
(Diff revision 1)
> +    const clientLogTimestamp = Date.now();
> +    const packet = await consoleAPICall(debuggerClient,
> +      () => top.console.log("test"));
> +
> +    const packetTimestamp = packet.message.timeStamp;
> +    const LOG_PROPAGATION_DELTA_MS = 1000;

I hope this isn't prone to intermittents, but you never know on debug builds. We could consider going back to 2000ms as the current test does, or instead taking another Date.now() measurement once packet has resolved and theoretically the timeStamp on the packet should be in between this. Up to you.

::: devtools/shared/webconsole/test/test_console_timestamp.html:44
(Diff revision 1)
> +  await closeDebugger(state);
> +  SimpleTest.finish();
> +};
> +
> +
> +function consoleAPICall(debuggerClient, consoleCall) {

Can you pull this into a helper function in common.js? Looks like we use this in two other places as well: https://searchfox.org/mozilla-central/search?q=addOneTimeListener%28%22consoleAPICall%22%29&path=
Attachment #8960201 - Flags: review?(bgrinstead) → review+
Comment on attachment 8960201 [details]
Bug 1405243 - Migrate browser_webconsole_bug_1006027_message_timestamps_incorrect.js to a server test; .

https://reviewboard.mozilla.org/r/228956/#review234696

> I hope this isn't prone to intermittents, but you never know on debug builds. We could consider going back to 2000ms as the current test does, or instead taking another Date.now() measurement once packet has resolved and theoretically the timeStamp on the packet should be in between this. Up to you.

oh yeah, sounds much more better

> Can you pull this into a helper function in common.js? Looks like we use this in two other places as well: https://searchfox.org/mozilla-central/search?q=addOneTimeListener%28%22consoleAPICall%22%29&path=

yes
Comment on attachment 8960247 [details]
Bug 1405243 - Extract consoleAPICall helper function in common.js; .

https://reviewboard.mozilla.org/r/229000/#review234758
Attachment #8960247 - Flags: review?(bgrinstead) → review+
Pushed by nchevobbe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3610b83bb375
Migrate browser_webconsole_bug_1006027_message_timestamps_incorrect.js to a server test; r=bgrins.
https://hg.mozilla.org/integration/autoland/rev/d4b5b83166e2
Extract consoleAPICall helper function in common.js; r=bgrins.
https://hg.mozilla.org/mozilla-central/rev/3610b83bb375
https://hg.mozilla.org/mozilla-central/rev/d4b5b83166e2
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Flags: needinfo?(nchevobbe)
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.