607113 - a bunch of api-utils tests fail

Status: RESOLVED FIXED

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

Description

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

Attachment: verbose test log

On trunk, a bunch of api-utils tests fail:

  test-api-utils.testPublicConstructor: failure
  test-api-utils.testValidateOptionsNonempty: failure
  test-api-utils.testValidateOptionsMap: failure
  test-api-utils.testValidateOptionsMapException: failure
  test-api-utils.testValidateOptionsOk: failure
  test-api-utils.testValidateOptionsIs: failure
  test-api-utils.testValidateOptionsMapIsOk: failure
  test-api-utils.testValidateOptionsErrorMsg: failure
  test-api-utils.testValidateMapWithMissingKey: failure
  test-api-utils.testAddIterator: failure, exception

Verbose test log attached.

It's not clear whether these are all due to the same issue or multiple (three?) different ones.

Drew: any ideas about these?

Comment 1

[Ed Lee :Mardak]

> error: fail: the keys/vals iterator returns the correct number of items (0 != 3)
This particular failure may be a dupe of bug 606573. If so, the fix for that bug has landed over the weekend, so could you try testing again with the latest tracemonkey build?

http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-tracemonkey/

Comment 2

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

Attachment: verbose test log against tracemonkey branch build

(In reply to comment #1)
> > error: fail: the keys/vals iterator returns the correct number of items (0 != 3)
> This particular failure may be a dupe of bug 606573. If so, the fix for that
> bug has landed over the weekend, so could you try testing again with the latest
> tracemonkey build?

More tests pass on the tracemonkey branch build (including the one you reference), but testPublicConstructor and testValidateOptionsIs still fail.

Comment 3

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

The known SDK blockers from the tracemonkey branch have landed on trunk, and more tests now pass on the latest trunk nightly, but the same test failures in comment 2 are still present on that nightly.

Drew: do you have any idea about the possible cause(s) of these last four test failures?

Comment 4

[Johnny Stenback (:jst)]

Myk, can you explain how this can be reproduced? Where should one pull from,
what needs to be built, etc etc?

Comment 5

[Drew Willcoxon :adw]

I just filed bug 607863 against the JS engine.  I haven't checked yet but I think the array error is different.  We can work around the other error about prototypes.

Comment 6

[Drew Willcoxon :adw]

Aha!  The array errors are caused by bug 606875, which just landed on mozilla-central today.  Thanks jst!  \o/

Actually, we should be using Array.isArray now rather than the hacky toString check.

Comment 7

[Drew Willcoxon :adw]

Attachment: patch

We can fix all the errors.  As comment 6 says, getTypeOf should use Array.isArray rather than the hacky toString check anyway, so let's fix it.  The isPrototypeOf changes that this patch makes are less strict than the assertions they replace, but I'm OK with that since api-utils clients (and clients of Jetpack APIs who are themselves clients of api-utils) generally should not be using __proto__s of the objects created from ctors returned by publicConstructor.  They should use instanceof and isPrototypeOf instead, and those still work.  (I checked our own code, and it doesn't appear that any of it relies on such __proto__s.)

Comment 8

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

Comment on attachment 486552 [details] [diff] [review]
patch

Looks good, fixes test failures; ship it! r=myk

Comment 9

[Drew Willcoxon :adw]

http://github.com/mozilla/addon-sdk/commit/15adc31b3ea69099e109cee9a7027f7d87c1bfc9

All problems fixed with this patch, so marking bug fixed.  I left the dependency on bug 607863 so it's clear what broke.

Comment 10

[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".