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

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P1)

Description

[Jorg K (CEST = GMT+2)]

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

Comment 1

[Jorg K (CEST = GMT+2)]

Seems to be caused by:
[JavaScript Warning: "ReferenceError: reference to undefined property Services.appinfo.name" {file: "resource://devtools/shared/Loader.jsm" line: 326}]

Comment 2

[Stefan Sitter [:ssitter]]

Seems similar to Bug 1269445. Maybe those tests need a similar fix.

Comment 3

[Stefan Sitter [:ssitter]]

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?

Comment 4

[aleth [:aleth]]

(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.

Comment 5

[aleth [:aleth]]

(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?

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

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.

Comment 7

[aleth [:aleth]]

(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.

Comment 8

[aleth [:aleth]]

(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.

Comment 9

[aleth [:aleth]]

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?

Comment 10

[aleth [:aleth]]

(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.

Comment 11

[Eddy Bruel [:ejpbruel]]

(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.

Comment 12

[aleth [:aleth]]

(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!

Comment 14

[Jorg K (CEST = GMT+2)]

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 ;-)

Comment 15

[aleth [:aleth]]

It's not quite the same error - the missing file is now devtools/client/shared/css-parsing-utils.

Comment 16

[Eddy Bruel [:ejpbruel]]

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.

Comment 17

[aleth [:aleth]]

(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.

Comment 18

[aleth [:aleth]]

Looks like this is due to
https://dxr.mozilla.org/mozilla-central/source/devtools/shared/fronts/styles.js#22
https://dxr.mozilla.org/mozilla-central/source/devtools/server/actors/styles.js#12

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

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.

Comment 20

[Patrick Brosset <:pbro>]

Attachment: MozReview Request: Bug 1270186 - Move css-parsing-utils.js from devtools/client to devtools/shared; r=tromey

Review commit: https://reviewboard.mozilla.org/r/54604/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/54604/

Comment 21

[Patrick Brosset <:pbro>]

(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.

Comment 22

[aleth [:aleth]]

This fixes the test locally for me with MOZ_DEVTOOLS="server" too. Thanks!

Comment 23

[Patrick Brosset <:pbro>]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c6198cd1b1e&group_state=expanded

Comment 24

[Tom Tromey :tromey]

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 :)

Comment 25

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/3531b8206f70

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3531b8206f70

Comment 27

[Liz Henry (:lizzard) (relman/hg->git project)]

Already resolved, just tracking for 49 in case it reopens.