Closed Bug 1270186 Opened 4 years ago Closed 4 years ago

TEST-UNEXPECTED-FAIL | devtools/server/tests/unit/test_animation_type.js and test_getRuleText.js and test_getTextAtLineColumn.js and test_nodelistactor.js

Categories

(DevTools :: Inspector, defect, P1)

defect

Tracking

(firefox49+ fixed)

RESOLVED FIXED
Firefox 49
Iteration:
49.3 - Jun 6
Tracking Status
firefox49 + fixed

People

(Reporter: jorgk, Assigned: pbro)

References

Details

(Keywords: regression)

Attachments

(1 file)

Fresh xpcshell bustage first seen May 4th, 2016:

TEST-UNEXPECTED-FAIL | devtools/server/tests/unit/test_animation_type.js | xpcshell return code: 0
ReferenceError: can't access lexical declaration `ANIMATION_TYPES' before initialization at C:/slave/test/build/tests/xpcshell/tests/devtools/server/tests/unit/test_animation_type.js:48
TEST-UNEXPECTED-FAIL | devtools/server/tests/unit/test_animation_name.js | xpcshell return code: 0
ReferenceError: can't access lexical declaration `AnimationPlayerActor' before initialization at C:/slave/test/build/tests/xpcshell/tests/devtools/server/tests/unit/test_animation_name.js:84
TEST-UNEXPECTED-FAIL | devtools/server/tests/unit/test_getRuleText.js | xpcshell return code: 0
ReferenceError: can't access lexical declaration `TEST_DATA' before initialization at C:/slave/test/build/tests/xpcshell/tests/devtools/server/tests/unit/test_getRuleText.js:113
TEST-UNEXPECTED-FAIL | devtools/server/tests/unit/test_getTextAtLineColumn.js | xpcshell return code: 0
ReferenceError: can't access lexical declaration `TEST_DATA' before initialization at C:/slave/test/build/tests/xpcshell/tests/devtools/server/tests/unit/test_getTextAtLineColumn.js:28
TEST-UNEXPECTED-FAIL | devtools/server/tests/unit/test_nodelistactor.js | xpcshell return code: 0
ReferenceError: can't access lexical declaration `NodeListActor' before initialization at C:/slave/test/build/tests/xpcshell/tests/devtools/server/tests/unit/test_nodelistactor.js:19
Version: 38 Branch → Trunk
Seems to be caused by:
[JavaScript Warning: "ReferenceError: reference to undefined property Services.appinfo.name" {file: "resource://devtools/shared/Loader.jsm" line: 326}]
Seems similar to Bug 1269445. Maybe those tests need a similar fix.
Should we move this to firefox - devtools component to get people involved that are familiar with the failing devtools tests? Or do you know someone we can add to CC and ask?
(In reply to Stefan Sitter from comment #3)
> Should we move this to firefox - devtools component to get people involved
> that are familiar with the failing devtools tests? Or do you know someone we
> can add to CC and ask?

TB doesn't ship devtools, we can simply disable these tests.
(In reply to aleth [:aleth] from comment #4)
> (In reply to Stefan Sitter from comment #3)
> > Should we move this to firefox - devtools component to get people involved
> > that are familiar with the failing devtools tests? Or do you know someone we
> > can add to CC and ask?
> 
> TB doesn't ship devtools, we can simply disable these tests.

Hmm, that's unless remote debugging depends on this code?
Flags: needinfo?(philipp)
The server tests should run, this is what remote debugging depends on. The client tests should be disabled, but they probably already are the way we build it.
Flags: needinfo?(philipp)
(In reply to Stefan Sitter from comment #2)
> Seems similar to Bug 1269445. Maybe those tests need a similar fix.

I think you're right, these tests fail locally as well with missing-file errors.
(In reply to aleth [:aleth] from comment #7)
> (In reply to Stefan Sitter from comment #2)
> > Seems similar to Bug 1269445. Maybe those tests need a similar fix.
> 
> I think you're right, these tests fail locally as well with missing-file
> errors.

On the other hand, the same tests pass when run locally on a m-c build.

Probably a TB-specific regression from bug 1268461 or bug 1270173.
ERROR Error: Module `devtools/client/fronts/stylesheets` is not found at resource://devtools/client/fronts/stylesheets.js at resource://devtools/server/actors/stylesheets.js:18

It seems that since bug 1268461, the server depends on devtools/client/fronts/stylesheets.js. The tests pass if I comment out https://dxr.mozilla.org/comm-central/source/mozilla/devtools/moz.build#10.

So it looks like any gecko app that uses remote debugging will be broken by this.

Looks like the moz.builds need some tweaking?
Flags: needinfo?(ejpbruel)
Component: General → Developer Tools: Inspector
Keywords: regression
Product: Thunderbird → Firefox
Blocks: 1268461
(In reply to aleth [:aleth] from comment #9)
> Looks like the moz.builds need some tweaking?

A better solution might be to move devtools/client/fronts/stylesheets.js to devtools/shared.
Whiteboard: [devtools-html] [triage]
Blocks: 1263289
(In reply to aleth [:aleth] from comment #10)
> (In reply to aleth [:aleth] from comment #9)
> > Looks like the moz.builds need some tweaking?
> 
> A better solution might be to move devtools/client/fronts/stylesheets.js to
> devtools/shared.

I have a patch ready in bug 1265722 that does just that.
Flags: needinfo?(ejpbruel)
(In reply to Eddy Bruel [:ejpbruel] from comment #11)
> > A better solution might be to move devtools/client/fronts/stylesheets.js to
> > devtools/shared.
> 
> I have a patch ready in bug 1265722 that does just that.

Great, thanks!
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
Whiteboard: [devtools-html] [triage]
Duplicate of bug: 1265722
Bug 1265722 landed today, 2016-05-19 15:37:27 CEST but I'm still seeing the test failures at Thu May 19, 19:43:22:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=45c77ad60f282ba44016332c5e4a7a56adea3d38
Click on the X and see:
TEST-UNEXPECTED-FAIL | devtools/server/tests/unit/test_animation_type.js
as originally reported.

So this isn't fixed as far as I can see. I'd love to be corrected ;-)
Status: RESOLVED → REOPENED
Flags: needinfo?(mmucci)
Flags: needinfo?(ejpbruel)
Resolution: DUPLICATE → ---
Flags: needinfo?(mmucci)
Whiteboard: [devtools-html] [triage]
It's not quite the same error - the missing file is now devtools/client/shared/css-parsing-utils.
As aleth pointed out in comment 15, the missing file now seems to be devtools/client/shared/css-parsing-utils. I didn't touch that file during our refactor, and as far as I can tell, it's right where it should be on my latest pull from fx-team.

I'm not sure why the tests complain about that file not being found, but as far as I can tell, this time it's not caused by any of my changes.
Flags: needinfo?(ejpbruel)
(In reply to Eddy Bruel [:ejpbruel] from comment #16)
> I'm not sure why the tests complain about that file not being found, but as
> far as I can tell, this time it's not caused by any of my changes.

The problem is that devtools/server and its tests should not depend on devtools/client. Otherwise there may be problems with remote debugging.
Flags: needinfo?(ejpbruel)
Status: REOPENED → NEW
Flags: qe-verify-
Priority: -- → P2
Whiteboard: [devtools-html] [triage] → [devtools-html]
I think the real culprit is bug 1069829, which first added the dependency on devtools/client/shared/css-parsing-utils.js to actor/styles.js.

:pbro, can you take a look?  Files in devtools/server can't depend on things from devtools/client since devtools/client is not shipped to all devices.  If the actor needs this module, we should move it devtools/shared.
Blocks: 1069829
No longer blocks: 1268461
Flags: needinfo?(ejpbruel) → needinfo?(pbrosset)
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #19)
> I think the real culprit is bug 1069829, which first added the dependency on
> devtools/client/shared/css-parsing-utils.js to actor/styles.js.
> 
> :pbro, can you take a look?  Files in devtools/server can't depend on things
> from devtools/client since devtools/client is not shipped to all devices. 
> If the actor needs this module, we should move it devtools/shared.
Thanks for the investigation. Looks like a simple file move then. I don't see any reason why css-parsing-utils.js couldn't be in devtools/shared, so I've moved it.
Try is closed right now, but tests pass fine locally. I'll push when it re-opens.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Flags: needinfo?(pbrosset)
This fixes the test locally for me with MOZ_DEVTOOLS="server" too. Thanks!
Comment on attachment 8755400 [details]
MozReview Request: Bug 1270186 - Move css-parsing-utils.js from devtools/client to devtools/shared; r=tromey

https://reviewboard.mozilla.org/r/54604/#review51808

It seems like we just did the move the other direction :)
Attachment #8755400 - Flags: review?(ttromey) → review+
https://hg.mozilla.org/mozilla-central/rev/3531b8206f70
Status: ASSIGNED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Iteration: --- → 49.3 - Jun 6
Priority: P2 → P1
Blocks: 1277706
No longer blocks: 1263289
Blocks: 1263289
Already resolved, just tracking for 49 in case it reopens.
Whiteboard: [devtools-html]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.