Closed Bug 1264679 Opened 4 years ago Closed 3 years ago

[rep tests] Add tests for date-time rep

Categories

(DevTools :: Shared Components, enhancement, P1)

enhancement

Tracking

(firefox50 fixed)

RESOLVED FIXED
Firefox 50
Iteration:
50.1 - Jun 20
Tracking Status
firefox50 --- fixed

People

(Reporter: linclark, Assigned: linclark)

References

Details

(Whiteboard: [devtools-html])

Attachments

(1 file, 2 obsolete files)

See Bug 1257552
Blocks: 1257552
Severity: normal → enhancement
Whiteboard: [devtools-html]
Flags: qe-verify-
Priority: -- → P2
Assignee: nobody → lclark
Status: NEW → ASSIGNED
Iteration: --- → 49.3 - Jun 6
Priority: P2 → P1
Status: ASSIGNED → NEW
Iteration: 49.3 - Jun 6 → ---
Priority: P1 → P2
The webconsole uses toISOString for datetime instead of toString. (See https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/console-output.js#2726) 


Would it be ok to change this?
Flags: needinfo?(odvarko)
Honza confirmed in meeting, it's fine to use ISO.
Flags: needinfo?(odvarko)
Attached patch Bug1264679.patch (obsolete) — Splinter Review
Attachment #8763622 - Flags: review?(odvarko)
Status: NEW → ASSIGNED
Iteration: --- → 50.1
Priority: P2 → P1
Lin, the patch doesn't include the test_reps_date-time.html test file.

Honza
Flags: needinfo?(lclark)
Attached patch Bug1264679.patch (obsolete) — Splinter Review
Sorry about that. This one includes it
Attachment #8763622 - Attachment is obsolete: true
Attachment #8763622 - Flags: review?(odvarko)
Flags: needinfo?(lclark)
Attachment #8763845 - Flags: review?(odvarko)
Comment on attachment 8763845 [details] [diff] [review]
Bug1264679.patch

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

One inline comment, but otherwise looks good to me!

Honza

::: devtools/client/shared/components/test/mochitest/test_reps_date-time.html
@@ +16,5 @@
> +<script type="application/javascript;version=1.8">
> +window.onload = Task.async(function* () {
> +  try {
> +    let ReactDOM = browserRequire("devtools/client/shared/vendor/react-dom");
> +    let React = browserRequire("devtools/client/shared/vendor/react");

I think you can remove these two dependencies.
Honza
Attachment #8763845 - Flags: review?(odvarko) → review+
Attached patch Bug1264679.patchSplinter Review
You are correct, thanks!
Attachment #8763845 - Attachment is obsolete: true
Attachment #8763950 - Flags: review+
Keywords: checkin-needed
This patch had the wrong commit message on it...
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/edcca1bcdb79
[rep tests] Add tests for date-time rep. r=mayhemer
The commit message is still wrong. The reviewer should be Honza (not mayhemer)

Honza
https://hg.mozilla.org/mozilla-central/rev/edcca1bcdb79
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 50
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.