1246730 - Add schema test.json

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: patch

I kinda cobbled this together from a Chromium schema and based on how we use browser.test.

Comment 1

[Kris Maglione [:kmag]]

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

Review of attachment 8717099 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/extensions/schemas/test.json
@@ +37,5 @@
> +        "description": "Sends a string message to the browser process, generating a Notification that C++ test code can wait for.",
> +        "allowAmbiguousOptionalArguments": true,
> +        "parameters": [
> +          {"type": "any", "name": "arg1", "optional": true},
> +          {"type": "any", "name": "arg2", "optional": true}

I guess we need a way to specify rest args...

@@ +62,5 @@
> +          {
> +            "name": "test",
> +            "choices": [
> +              {"type": "string"},
> +              {"type": "boolean"}

We should probably accept integers and null, too. Same for assertFalse.

Although, I wonder if checking the type is going to lead to incorrect results if a test winds up passing an object, when it's expecting something else, and the test function throws rather than reporting failure?

@@ +105,5 @@
> +        "unsupported": true,
> +        "allowAmbiguousOptionalArguments": true,
> +        "parameters": [
> +          {"type": "any", "name": "expected", "optional": true},
> +          {"type": "any", "name": "actual", "optional": true}

Should the first two args really be optional (same for assertEq)? Does type=any not allow us to pass explicit null/undefined?

@@ +129,5 @@
> +        "name": "assertLastError",
> +        "type": "function",
> +        "unsupported": true,
> +        "parameters": [
> +          {"type": "string", "name": "expectedError"}

Might be good to accept the same types as assertThrows here.

@@ +144,5 @@
> +            "name": "self",
> +            "additionalProperties": {"type": "any"},
> +            "optional": true
> +          },
> +          {"type": "array", "items": {"type": "any"}, "name": "args"},

I guess we're stuck accepting "self" and "args" if we want to match Chrome... They don't really seem necessary, though. If we're not passing a closure, we can always use .bind().

Would be nice to at least make "args" optional too.

@@ +145,5 @@
> +            "additionalProperties": {"type": "any"},
> +            "optional": true
> +          },
> +          {"type": "array", "items": {"type": "any"}, "name": "args"},
> +          {"choices": [ {"type": "string"}, {"type": "object", "isInstanceOf": "RegExp"} ], "name": "message", "optional": true}

Assert.throws allows a function that takes the error value and returns true or false, too. Not sure how useful that'd be here, though.

Comment 2

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ec234b78ab0a

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ec234b78ab0a