1674841 - Clean up devtools/shared/protocol/tests/xpcshell/test_protocol_async.js

Status: RESOLVED FIXED

(DevTools :: General, task)

Description

[Andrey Bienkowski]

The attached patch fixes the following:

1. Replace var with const where appropriate
2. Refactor promiseReturn() into an async function (because it makes it cleaner)
3. Refactor promiseThrow(). It was using a fixed timeout hoping that the timeout value is large enough to reproduce a race condition. In the initial version of this file both promiseThrow() and promiseReturn() were using timeouts. During code review promiseReturn() was changed to use do_execute_soon(), but promiseThrow() inexplicably was not. The two functions have so much in common that I reimplemented promiseThrow() as a wrapper around promiseReturn().
4. Remove usage of defer(). I believe that in this case there is no need to replace it with new Promise

Here is why I think it is safe to remove defer() here.

My best-effort summary of the history of the file:

1. A test is introduced to make sure responses are delivered in the same order as requests were sent
2. A quirk of Promise.jsm partially breaks the behavior the test was designed to test. A workaround is added to the test to silence it.

I am assuming that both the test and the code under test now use a standard compliant implementation of Promise. If my assumption is correct, I think the workaround is no longer necessary.

PS I tried to do my best but I'm only a human. It is quite possible that some of the above is incorrect

Comment 1

[Andrey Bienkowski]

Attachment: Bug 1674841 - [devtools] Clean up devtools/shared/protocol/tests/xpcshell/test_protocol_async.js r=nchevobbe

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/341abc5d6ef7
[devtools] Clean up devtools/shared/protocol/tests/xpcshell/test_protocol_async.js r=nchevobbe

Comment 3

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/341abc5d6ef7