566784 - Simple storage test fails when run by cfx testall

Status: RESOLVED FIXED

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

Description

[Drew Willcoxon :adw]

Attachment: patch

Crap, a weird (but cool) thing the simple storage test does to get at its module's sandbox breaks when run via cfx testall.  With this patch all tests really pass, cfx text or cfx testall.  Is there a better way to do this?  Is there a way to generate resource URIs given a filename or file URI?

Comment 1

[Dietrich Ayala (:dietrich)]

confirmed fixes the failure for me.

Comment 2

[Atul Varma [:atul]]

Oh man! I should've seen this when code reviewing simple-storage, my bad.

So, one option for a cleaner solution is to add a method to cuddlefish.Loader objects called findSandboxForModule().  This would turn your code into something like this:

function manager(loader) {
  return loader.findSandboxForModule("simple-storage").globalScope.manager;
}

The method would be trivial for me to implement; what do you think?

Comment 3

[Atul Varma [:atul]]

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

You can commit this for now if you want to solve the immediate problem of the busted tree, but I'd really like a solution that doesn't involve hard-coding these URLs, which is really an implementation detail that test suites shouldn't have to deal with (and dealing with them just makes them hella fragile).

Comment 4

[Drew Willcoxon :adw]

(In reply to comment #2)
> The method would be trivial for me to implement; what do you think?

That sounds great Atul.

Comment 5

[Atul Varma [:atul]]

Attachment: fix via new securablemodule.Loader.findSandboxForModule() method.

Comment 6

[Atul Varma [:atul]]

Comment on attachment 446253 [details] [diff] [review]
fix via new securablemodule.Loader.findSandboxForModule() method.

I know securable-module.js hasn't been code reviewed yet, but if you could just give this a quick look over it would be helpful.

Comment 7

[Drew Willcoxon :adw]

Comment on attachment 446253 [details] [diff] [review]
fix via new securablemodule.Loader.findSandboxForModule() method.

Looks OK to me, and tests pass with it, thanks!

Comment 8

[Atul Varma [:atul]]

Thanks! Pushed: http://hg.mozilla.org/labs/jetpack-sdk/rev/21537f905bf6

Comment 9

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