607768 - test-match-pattern fails three tests due to apparent regexp mismatches

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

On the latest Firefox trunk nightly, three tests are failing in match-pattern.

--------------------------------------------------------------------------------
(addon-sdk)myk@myk:~/Projects/addon-sdk/packages/jetpack-core$ cfx test -F match-pattern
Using binary at '/home/myk/bin/firefox'.
Using profile at '/tmp/tmpmO4BNZ.mozrunner'.

(firefox-bin:23280): GLib-WARNING **: g_set_prgname() called multiple times
..................................error: TEST FAILED: test-match-pattern.testMatchPatternErrors (failure)
error: fail: MatchPattern throws when supplied multiple '*' ("There can be at most one '*' character in a wildcard." != /There can be at most one/)
info: Traceback (most recent call last):
  File "resource://jetpack-core-jetpack-core-lib/timer.js", line 64, in notifyOnTimeout
    this._callback.apply(null, this._params);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 255, in anonymous
    timer.setTimeout(function() { onDone(self); }, 0);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 280, in runNextTest
    self.start({test: test, onDone: runNextTest});
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 298, in start
    this.test.testFunction(this);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 63, in runTest
    test(runner);
  File "resource://jetpack-core-jetpack-core-tests/test-match-pattern.js", line 103, in anonymous
    "MatchPattern throws when supplied multiple '*'"
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 188, in assertRaises
    this.assertEqual(errorMessage, predicate, message);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 227, in assertEqual
    this.fail(message);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 145, in fail
    console.trace();
error: fail: MatchPattern throws when the wildcard doesn't use '*' and doesn't look like a URL ("When not using *.example.org wildcard, the string supplied is expected to be either an exact URL to match or a URL prefix. The provided string ('google.com') is unlikely to match any pages." != /expected to be either an exact URL/)
info: Traceback (most recent call last):
  File "resource://jetpack-core-jetpack-core-lib/timer.js", line 64, in notifyOnTimeout
    this._callback.apply(null, this._params);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 255, in anonymous
    timer.setTimeout(function() { onDone(self); }, 0);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 280, in runNextTest
    self.start({test: test, onDone: runNextTest});
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 298, in start
    this.test.testFunction(this);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 63, in runTest
    test(runner);
  File "resource://jetpack-core-jetpack-core-tests/test-match-pattern.js", line 109, in anonymous
    "MatchPattern throws when the wildcard doesn't use '*' and doesn't " +
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 188, in assertRaises
    this.assertEqual(errorMessage, predicate, message);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 227, in assertEqual
    this.fail(message);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 145, in fail
    console.trace();
error: fail: MatchPattern throws when a '*' is in the middle of the wildcard ("The provided wildcard ('http://google*.com') has a '*' in an unexpected position. It is expected to be the first or the last character in the wildcard." != /expected to be the first or the last/)
info: Traceback (most recent call last):
  File "resource://jetpack-core-jetpack-core-lib/timer.js", line 64, in notifyOnTimeout
    this._callback.apply(null, this._params);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 255, in anonymous
    timer.setTimeout(function() { onDone(self); }, 0);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 280, in runNextTest
    self.start({test: test, onDone: runNextTest});
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 298, in start
    this.test.testFunction(this);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 63, in runTest
    test(runner);
  File "resource://jetpack-core-jetpack-core-tests/test-match-pattern.js", line 116, in anonymous
    "MatchPattern throws when a '*' is in the middle of the wildcard"
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 188, in assertRaises
    this.assertEqual(errorMessage, predicate, message);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 227, in assertEqual
    this.fail(message);
  File "resource://jetpack-core-jetpack-core-lib/unit-test.js", line 145, in fail
    console.trace();
...
37 of 40 tests passed.
FAIL
Total time: 1.513593 seconds
Program terminated unsuccessfully.
--------------------------------------------------------------------------------

The log messages very much resemble those in bug 607766, suggesting a common cause (f.e. a regression or perhaps an intentional change in the behavior of regexp pattern matching).

mrbkap: have any changes to JS regexp parsing landed since 4.0b6?

Comment 1

[Drew Willcoxon :adw]

Blake, the problem seems to be that typeof(regexp) outside a sandbox returns "function" when regexp was created in the sandbox:

  var sp = Components.classes["@mozilla.org/systemprincipal;1"].
           createInstance(Components.interfaces.nsIPrincipal);
  var s = Components.utils.Sandbox(sp);
  Components.utils.evalInSandbox("re = /foo/;", s);
  typeof(s.re); /* "function */

Comment 2

[Drew Willcoxon :adw]

I filed bug 607799 in Core::JavaScript engine and CC'ed Blake.

Comment 3

[Drew Willcoxon :adw]

I should also note that it would be really easy for us to fix these failing tests even with the deeper JS engine bug: assertRaises() in unit-test.js checks that typeof(predicate) == "object" to see if predicate is a regexp, but it could just as easily flip the conditional and check if predicate is a string.

Comment 4

[Drew Willcoxon :adw]

Attachment: patch

Since bug 607799 has been marked blocking b8, this patch does what comment 3 says and will hold us over in the meantime.  Actually this change is just as correct as what the code is currently doing, so it's not a hack or anything.

Comment 5

[Drew Willcoxon :adw]

Fixed in 0.10:

http://github.com/mozilla/addon-sdk/commit/fc105c3396764f9b98de90ae2a9f29b388915d84

I'll leave the dependency on bug 607799 so it's clear what broke.

Comment 6

[Myk Melez [:myk] [@mykmelez]]

The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".