Closed Bug 1313765 Opened 3 years ago Closed 3 years ago

Make Reps work with Proxy

Categories

(DevTools :: Shared Components, defect)

defect
Not set

Tracking

(firefox52 fixed)

RESOLVED FIXED
Firefox 52
Tracking Status
firefox52 --- fixed

People

(Reporter: ntim, Assigned: nchevobbe)

References

Details

Attachments

(1 file)

The old output does that without any specific rep for Proxy, so it might be worth investigating why the default rep currently does not work for Proxy.

var p = new Proxy({}, () => {}); p;
Assignee: nobody → chevobbe.nicolas
Comment on attachment 8806149 [details]
Bug 1313765 - Make Rep handles Proxy. ;

https://reviewboard.mozilla.org/r/89644/#review89290

I am seeing a test failure on Try:

TEST-UNEXPECTED-FAIL | devtools/client/shared/components/test/mochitest/test_reps_event.html | Event rep has expected text content for a message event - got "MessageEvent { isTrusted: false, data: \"test data\", 7 more… }", expected "MessageEvent { isTrusted: false, data: \"test data\", origin: \"null\", 7 more… }"

Why the 'origin' field is now missing?


Honza
Attachment #8806149 - Flags: review?(odvarko)
I made some false assumptions and it's why this test fails. I fixed it, and ran the whole reps' test suite, which now pass.

One thing bothers me, aren't the Reps' tests running on TRY ? I have a run where I can't see any error on the test, which indeed was failing locally https://treeherder.mozilla.org/#/jobs?repo=try&revision=cffdbeccf2fc .
Comment on attachment 8806149 [details]
Bug 1313765 - Make Rep handles Proxy. ;

https://reviewboard.mozilla.org/r/89644/#review89590

R+ assuming try is green.

Thanks!
Honza
Attachment #8806149 - Flags: review?(odvarko) → review+
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #4)
> One thing bothers me, aren't the Reps' tests running on TRY ? I have a run
> where I can't see any error on the test, which indeed was failing locally
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=cffdbeccf2fc .

This is weird they should be running on Try.
Here is a test push with an error, let's see if it's visible.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7eaba1987502d4630e93c29e48f6859a78f55790

Honza
(In reply to Jan Honza Odvarko [:Honza] from comment #6)
> (In reply to Nicolas Chevobbe [:nchevobbe] from comment #4)
> > One thing bothers me, aren't the Reps' tests running on TRY ? I have a run
> > where I can't see any error on the test, which indeed was failing locally
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=cffdbeccf2fc .
> 
> This is weird they should be running on Try.
> Here is a test push with an error, let's see if it's visible.
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=7eaba1987502d4630e93c29e48f6859a78f55790
> 
> Honza

The TRY run you point too contains the fixed version of the patch ( https://hg.mozilla.org/try/diff/4241815ac1ad/devtools/client/shared/components/reps/grip.js ) and not the faulty one ( https://hg.mozilla.org/try/diff/006f54eaa79c/devtools/client/shared/components/reps/grip.js ), so, if I did my job right, we should not see jobs failing. Anyway, I'll check in the raw logs of dt-* tests to check if reps tests where executed, which does not seem the case in https://treeherder.mozilla.org/#/jobs?repo=try&revision=cffdbeccf2fc .
Ah, I did an explicit change in testProxy() method (modified & wrong defaultOutput), but it didn't propagate to Try.

Here is a new push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=78525cf44f50b5529e4fcf2b1a1312b279770a74

I am curious to see what happens.
Honza
From IRC:

linclark Honza: nchevobbe I think they are in the oth suite
linclark Honza: mochitest-dt only runs browser mochitests IIRC


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