proposal: Add amendments to browser.test API (#1051)

[justinlulejian]

No description provided.

[xeenon]

This differs from what we do in Safari/WebKit. browser.test.succeed(message) is just an alias for assertTrue(true, message). We have browser.test.notifyPass(message) as the "marks the current test as passed" method. IIRC, this is what Chrome and Firefox did too last I checked when we added these.

Should WebKit have succeed alias to notifyPass instead?

[justinlulejian]

In Chrome browser.test.notifyPass/browser.test.notifyFail are for the whole suite of browser.test.runTests, whereas browser.test.succeed in for the specific test case it's run in. For Safari, is browser.test.notifyPass/browser.test.notifyFail for the whole suite of tests or each individual test case? If the latter, then yes you could have browser.test.succeed/browser.test.fail just alias to browser.test.notifyPass/browser.test.notifyFail.

[kiaraarose]

In Chrome browser.test.notifyPass/browser.test.notifyFail are for the whole suite of browser.test.runTests

what about outside the context of runTests? how does notifyPass/notifyFail behave?

[kiaraarose]

IIRC, this is what Chrome and Firefox did too last I checked when we added these.

Firefox also does an alias: https://searchfox.org/firefox-main/source/toolkit/components/extensions/child/ext-test.js#299-305

[xeenon]

Similar to https://github.com/w3c/webextensions/pull/1057/changes#r3724127353. We have browser.test.fail(message) as an alias for assertTrue(false, message). browser.test.notifyFail(message) is "marks the current test as failed" method.

Should WebKit have fail alias to notifyFail instead?

[justinlulejian]

See above.

[xeenon]

WebKit currently has isProcessingUserGesture. We will need to add support for the new name before we can adopt this in WPT.

[justinlulejian]

I had switched this since it was suggested as a good alternative. Chrome also implements it as isProcessingUserGesture. Do other browser have a strong position on this? Otherwise I'm happy saving the effort and keeping it as isProcessingUserGesture.

[kiaraarose]

i'm fine with isUserGestureActive. but as Tim said, we would need to adopt in WebKit and wait for that change to be picked up in Safari Tech Preview before using it in the wpt test

[Rob--W]

Should we document a return value (whatever the function returns?). I mentioned that at the last meeting and #1051 (comment)

I don't mind not returning anything, but then someone always needs to do something like:

await new Promise(resolve => {
  browser.test.runWithUserGesture(
    () => resolve(browser.permissions.request(perms))
  );
});

Opposed to:

await browser.test.runWithUserGesture(
  () => browser.permissions.request(perms)
);

FWIW, our (differently named) test helper in Firefox does currently NOT return the value, to discourage the reader from thinking (incorrectly) that returning a promise would extend the user gesture beyond the current run of the event loop.

[justinlulejian]

Added a returns section to this method. Chrome usually combines it internally with chrome.test.succeed internally so that's why I omitted it, but I agree it's more ergonomic to the function's result to not have to wrap it in a promise each time. The refactor and impl would be pretty simple to implement in Chrome.

[kiaraarose]

We return the result in WebKit