1038381 - Replace usage of @test/options with sdk/test/options

Status: RESOLVED FIXED

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

Description

[Erik Vold [:erikvold] (please needinfo? me)]

At the moment we have various modules that rely on a special `@test/options` module that is created in the bootstrap file.  JPM now sets prefs instead of using this `@test/options` module that was created in cfx development, so I suggest that we create a new module called `sdk/test/options` which pulls the information that `@test/options` used to provide, from `@test/options` if it exists, and then from the prefs which JPM now uses.

Comment 1

[Dave Townsend [:mossop]]

The thing I like about the @ syntax is that it makes it clear that this is something internal that you shouldn't use in general

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Dave Townsend [:mossop] from comment #1)
> The thing I like about the @ syntax is that it makes it clear that this is
> something internal that you shouldn't use in general

it is also confusing to figure out where it comes from and how to make changes to it though.

Comment 3

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Do we need a module to capture all the options though ? Or in other words do we have multiple places where we are going to read those options ? If it's just test runner I'd rather inline that there.

I would also rather have `readTestOptions` in the test/runner or some other test related module that will take care of reading and normalizing.

Finally I think prefs should take precedence over `@test/options` unless there is a good reason to do otherwise which I fail to see.

Comment 4

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #3)
> Do we need a module to capture all the options though ? Or in other words do
> we have multiple places where we are going to read those options ? If it's
> just test runner I'd rather inline that there.

There are 4 places at the moment, excluding tests https://github.com/mozilla/addon-sdk/search?p=1&q=%22%40test%2Foptions%22%2F%27%40test%2Foptions%27&type=Code

> I would also rather have `readTestOptions` in the test/runner or some other
> test related module that will take care of reading and normalizing.

I'm not sure what you mean here.  Perhaps you were assuming there was only one module that uses "@test/options"?

> Finally I think prefs should take precedence over `@test/options` unless
> there is a good reason to do otherwise which I fail to see.

sounds fine to me

Comment 5

[Erik Vold [:erikvold] (please needinfo? me)]

Attachment: Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1563

Comment 6

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/a106bfc8d9f5eadd4920489f33916464ee5711e8
Bug 1038381 - Replace usage of @test/options with sdk/test/options

https://github.com/mozilla/addon-sdk/commit/b6b476ed17e90e39ab45841795eaba4abdc68182
Merge pull request #1563 from erikvold/1038381

Bug 1038381 - Replace usage of @test/options with sdk/test/options r=@gozala