562886 - test_plugins.js fails (8 == 4) with Linux objdir builds

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect, P1)

Description

[Mark Banner (:standard8)]

From the add-on manager landing, we're getting failures in test_plugins.js:

The interesting bit:

TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 22] true == true
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 29] {4959e6e7-8218-4c84-bcc6-cc0c52fcb95e} != null
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 32] [object Object] != null
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 33] Test Plug-in == Test Plug-in
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 34] Plug-in for testing purposes. == Plug-in for testing purposes.
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 35]  == 
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 36] 1.0.0.0 == 1.0.0.0
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 37] plugin == plugin
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 38] false == false
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 39] false == false
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 40] true == true
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 41] true == true
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 42] true == true
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 43] 0 == 0
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 44] 4 == 4
TEST-PASS | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | [anonymous : 45] 0 == 0
TEST-UNEXPECTED-FAIL | /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js | 8 == 4 - See following stack:
JS frame :: /buildbot/linux-comm-central-check/build/mozilla/testing/xpcshell/head.js :: do_throw :: line 257
JS frame :: /buildbot/linux-comm-central-check/build/mozilla/testing/xpcshell/head.js :: do_check_eq :: line 287
JS frame :: /buildbot/linux-comm-central-check/build/objdir/mozilla/_tests/xpcshell/test_extensionmanager/xpcshell/test_plugins.js :: anonymous :: line 46
JS frame :: file:///buildbot/linux-comm-central-check/build/objdir/mozilla/dist/bin/modules/AddonManager.jsm :: safeCall :: line 41
JS frame :: file:///buildbot/linux-comm-central-check/build/objdir/mozilla/dist/bin/modules/AddonManager.jsm :: anonymous :: line 565

At a quick glance I can't see why Thunderbird linux builds would have SCOPE_SYSTEM, but its a persistent failure.

So any thoughts on what to look at would be useful.

Comment 1

[:Mook]

From your log, it looks like you're running the tests in the objdir (i.e. from dist/bin).  From the tinderbox logs, it looks like that's running packaged tests (i.e. the app was extracted from a  tarball).

Looking at the relevant code, http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/PluginProvider.jsm?rev=a4a1332487ea&mark=226#208 :

Dumping the paths I get when running the test in my objdir, I see that:
path    = /src/mozilla/obj/browser-debug/modules/plugin/test/testplugin/libnptest.so
APlugns = /src/mozilla/obj/browser-debug/dist/bin/plugins

In the c-c case, the plugin file (dist/bin/plugins/libnptest.so) is a symlink; in the m-c case, it's a real file because libnptest.so got there via firefox-*.tests.zip which can't hold symlinks.  nsIPluginTag.fullpath resolves symlinks due to the JRE:
http://mxr.mozilla.org/mozilla-central/source/modules/plugin/base/src/nsPluginHost.cpp?rev=a4a1332487ea&mark=3320-3320#3282

(Commenting here because I also got bitten due to the same problem... surprised nobody else seems to be hitting this.)

Comment 2

[Mark Banner (:standard8)]

Ok, so I think its now clear that this test fails with Linux objdir builds.

(philor has also told me this was seen on try server).

Prompting the severity because if this is going to fail in objdir builds, then this will affect any dev that wants to try and run tests locally.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1273595677.1273605056.25014.gz
Linux try hg unit test on 2010/05/11 09:34:37

Comment 4

[Steffen Wilberg]

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1273605867.1273617161.6340.gz
Linux try hg unit test on 2010/05/11 12:24:27

Comment 5

[Dave Townsend [:mossop]]

Crib notes for a fix in case someone else has a moment to get to this before I do:

We should be able to find the actual test plugin file using the code in http://mxr.mozilla.org/mozilla-central/source/modules/plugin/test/unit/head_plugins.js. Adjust the scope we check for based on whether that file is a link or not.

Comment 6

[Zack Weinberg (:zwol)]

FYI, this bites the try server as well, and the failed assertion doesn't always seem to have the same numbers on either side; I just got 4 == 8 and 2 == 4.

Comment 7

[Dave Townsend [:mossop]]

(In reply to comment #6)
> FYI, this bites the try server as well, and the failed assertion doesn't always
> seem to have the same numbers on either side; I just got 4 == 8 and 2 == 4.

That seems very strange, can you give a log file where that happened?

Comment 8

[Zack Weinberg (:zwol)]

Attachment: logfile where test fails with 2 == 4

(In reply to comment #7)
> (In reply to comment #6)
> > FYI, this bites the try server as well, and the failed assertion doesn't always
> > seem to have the same numbers on either side; I just got 4 == 8 and 2 == 4.
> 
> That seems very strange, can you give a log file where that happened?

Here it is.

Comment 9

[Dave Townsend [:mossop]]

Ah nice, you're building in your home directory so the manager things it is a user installed plugin. Need some tweaking to solve that...

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1273793787.1273803242.22684.gz
Linux try hg unit test on 2010/05/13 16:36:27

Comment 11

[Jason Orendorff [:jorendorff]]

http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1274220651.1274231502.21114.gz
Linux try hg unit test on 2010/05/18 15:08:49

Comment 12

[Mark Banner (:standard8)]

Attachment: Fix for the Linux objdir test case

This should fix the linux objdir testcase which we're seeing in Thunderbird and the try server.

I've just pushed it to try server but it works for me locally on Mac and Linux objdir builds.

It doesn't attempt to resolve the 2 == 4 issue as I don't have my builds set up for that, and the 8 == 4 issue is the main one lots of people are seeing.

Comment 13

[Mark Banner (:standard8)]

Comment on attachment 446259 [details] [diff] [review]
Fix for the Linux objdir test case

This seems to work fine - try server builds passed with it.

Like I said in the previous comment, I haven't got the set up to reproduce the 2 == 4 version, so I'm not going to try and fix that. This will at least fix the Thunderbird tinderboxes and try server.

Comment 14

[Dave Townsend [:mossop]]

Comment on attachment 446259 [details] [diff] [review]
Fix for the Linux objdir test case

>diff -r 9fbcdeef28c0 toolkit/mozapps/extensions/test/xpcshell/test_plugins.js
>--- a/toolkit/mozapps/extensions/test/xpcshell/test_plugins.js	Tue May 18 09:50:14 2010 +0200
>+++ b/toolkit/mozapps/extensions/test/xpcshell/test_plugins.js	Wed May 19 19:27:31 2010 +0100
>@@ -16,6 +16,21 @@ function run_test() {
>   run_test_1();
> }
> 
>+function get_unix_test_plugin() {
>+  let plugins = Components.classes["@mozilla.org/file/directory_service;1"].
>+                getService(Components.interfaces.nsIProperties).
>+                get("CurProcD", Components.interfaces.nsILocalFile);
>+  plugins.append("plugins");
>+  let plugin = plugins.clone();
>+  // *nix plugin
>+  plugin.append("libnptest.so");
>+  if (plugin.exists()) {
>+    plugin.normalize();
>+    return plugin;
>+  }
>+  return null;
>+}
>+
> // Tests that the test plugin exists
> function run_test_1() {
>   AddonManager.getAddonsByTypes("plugin", function(addons) {
>@@ -29,7 +44,7 @@ function run_test_1() {
>     do_check_neq(gID, null);
> 
>     AddonManager.getAddonByID(gID, function(p) {
>-      do_check_neq(p, null)
>+      do_check_neq(p, null);
>       do_check_eq(p.name, "Test Plug-in");
>       do_check_eq(p.description, "Plug-in for testing purposes.");
>       do_check_eq(p.creator, "");
>@@ -43,7 +58,15 @@ function run_test_1() {
>       do_check_eq(p.blocklistState, 0);
>       do_check_eq(p.permissions, AddonManager.PERM_CAN_DISABLE);
>       do_check_eq(p.pendingOperations, 0);
>-      do_check_eq(p.scope, AddonManager.SCOPE_APPLICATION);
>+
>+      // Work around the fact that on Linux source builds, if we're using
>+      // symlinks (i.e. objdir), then Linux will see these as a different scope
>+      // to non-symlinks.
>+      let pluginLoc = get_unix_test_plugin();
>+      let pluginScope = AddonManager.SCOPE_APPLICATION;
>+      if (pluginLoc && pluginLoc.isSymlink)

isSymlink is a method not an attribute.

>+        pluginScope = AddonManager.SCOPE_SYSTEM;
>+      do_check_eq(p.scope, pluginScope);
>       do_check_true("isCompatibleWith" in p);
>       do_check_true("findUpdates" in p);
> 
>@@ -70,7 +93,7 @@ function run_test_2(p) {
>   do_check_false(p.isActive);
> 
>   AddonManager.getAddonByID(gID, function(p) {
>-    do_check_neq(p, null)
>+    do_check_neq(p, null);
>     do_check_true(p.userDisabled);
>     do_check_false(p.appDisabled);
>     do_check_false(p.isActive);

Comment 15

[Mark Banner (:standard8)]

Attachment: Fix for the Linux objdir test case v2

Sorry, not sure how I missed that, this one should work better.

Comment 16

[Dave Townsend [:mossop]]

Comment on attachment 446872 [details] [diff] [review]
Fix for the Linux objdir test case v2

Looks good, thanks

Comment 17

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/mozilla-central/rev/ec3dafe67d2b

I'd like to suggest that as we haven't yet got a patch or offer of help for the 2 == 4 bit, that we ship that out to a separate bug if it is still happening (I suspect it is).

Comment 18

[Zack Weinberg (:zwol)]

(In reply to comment #17)
> 
> I'd like to suggest that as we haven't yet got a patch or offer of help for the
> 2 == 4 bit, that we ship that out to a separate bug if it is still happening (I
> suspect it is).

Sure is.  Filed bug 568027.

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Marking as verified fixed based on no failures in the last couple months.