1089238 - TEST-UNEXPECTED-FAIL | addon-sdk/tests/test-page-mod.testContentScriptWhenOnTabReady | onComplete should not be called with 'start'.

Status: RESOLVED FIXED

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

Description

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

TEST-PASS | addon-sdk/tests/test-page-mod.testContentScriptWhenOnTabOpen | PageMod is attached when document is complete
[3883] WARNING: NS_ENSURE_TRUE(mMutable) failed: file /builds/slave/fx-team-l64-d-0000000000000000/build/netwerk/base/src/nsSimpleURI.cpp, line 265
[3883] WARNING: NS_ENSURE_TRUE(mCallback) failed: file /builds/slave/fx-team-l64-d-0000000000000000/build/content/base/src/nsFrameMessageManager.cpp, line 675
[3883] WARNING: NS_ENSURE_TRUE(mCallback) failed: file /builds/slave/fx-team-l64-d-0000000000000000/build/content/base/src/nsFrameMessageManager.cpp, line 675
TEST-END | addon-sdk/tests/test-page-mod.testContentScriptWhenOnTabOpen
TEST-START | addon-sdk/tests/test-page-mod.testContentScriptWhenOnTabReady
++DOCSHELL 0x7faffde21800 == 22 [pid = 3883] [id = 427]
++DOMWINDOW == 187 (0x7faffd7a0800) [pid = 3883] [serial = 1162] [outer = (nil)]
++DOMWINDOW == 188 (0x7faffdcda000) [pid = 3883] [serial = 1163] [outer = 0x7faffd7a0800]
[3883] WARNING: NS_ENSURE_TRUE(mMutable) failed: file /builds/slave/fx-team-l64-d-0000000000000000/build/netwerk/base/src/nsSimpleURI.cpp, line 265
++DOMWINDOW == 189 (0x7faffe394000) [pid = 3883] [serial = 1164] [outer = 0x7faffd7a0800]
TEST-INFO | [JavaScript Warning: "Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user's experience. For more help http://xhr.spec.whatwg.org/" {file: "resource://f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack/addon-sdk/tests/fixtures/test-contentScriptWhen.html?OnTabReady" line: 16}]
TEST-UNEXPECTED-FAIL | addon-sdk/tests/test-page-mod.testContentScriptWhenOnTabReady | onComplete should not be called with 'start'.
TEST-INFO | Traceback (most recent call last):
  File "resource://extensions.modules.f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack.commonjs.path/sdk/content/worker-parent.js", line 74, in receive
    emit(this, ...args);
  File "resource://extensions.modules.f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack.commonjs.path/sdk/event/core.js", line 97, in emit
    listener.apply(target, args);
  File "resource://f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack/addon-sdk/tests/pagemod-test-helpers.js", line 78, in exports.handleReadyState/pagemod<.onAttach/<
    callbacks[type](tab);
  File "resource://f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack/addon-sdk/tests/test-page-mod.js", line 721, in exports.testContentScriptWhenOnTabReady/<.onReady/<.onComplete
    onComplete: () => assert.fail("onComplete should not be called with 'start'."),
  File "resource://extensions.modules.f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack.commonjs.path/sdk/test/assert.js", line 73, in fail
    this._log.fail(e);
  File "resource://extensions.modules.f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack.commonjs.path/sdk/deprecated/unit-test.js", line 97, in fail
    this.console.testMessage(false, false, this.test.name, message);
  File "resource://extensions.modules.f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack.commonjs.path/sdk/test/harness.js", line 546, in testMessage
    this.trace();
TEST-UNEXPECTED-FAIL | addon-sdk/tests/test-page-mod.testContentScriptWhenOnTabReady | onComplete should not be called with 'ready'.
TEST-INFO | Traceback (most recent call last):
  File "resource://extensions.modules.f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack.commonjs.path/sdk/content/worker-parent.js", line 74, in receive
    emit(this, ...args);
  File "resource://extensions.modules.f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack.commonjs.path/sdk/event/core.js", line 97, in emit
    listener.apply(target, args);
  File "resource://f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack/addon-sdk/tests/pagemod-test-helpers.js", line 78, in exports.handleReadyState/pagemod<.onAttach/<
    callbacks[type](tab);
  File "resource://f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack/addon-sdk/tests/test-page-mod.js", line 731, in exports.testContentScriptWhenOnTabReady/<.onReady/<.onComplete
    onComplete: () => assert.fail("onComplete should not be called with 'ready'."),
  File "resource://extensions.modules.f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack.commonjs.path/sdk/test/assert.js", line 73, in fail
    this._log.fail(e);
  File "resource://extensions.modules.f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack.commonjs.path/sdk/deprecated/unit-test.js", line 97, in fail
    this.console.testMessage(false, false, this.test.name, message);
  File "resource://extensions.modules.f3e937c8-774c-4cc4-9336-3c6321da8d89-at-jetpack.commonjs.path/sdk/test/harness.js", line 546, in testMessage
    this.trace();
TEST-PASS | addon-sdk/tests/test-page-mod.testContentScriptWhenOnTabReady | PageMod is attached when document is complete
--DOCSHELL 0x7faffd81c000 == 21 [pid = 3883] [id = 416]
--DOCSHELL 0x7fb00065b800 == 20 [pid = 3883] [id = 419]
--DOCSHELL 0x7faffd87e800 == 19 [pid = 3883] [id = 411]
--DOCSHELL 0x7faffeb98800 == 18 [pid = 3883] [id = 406]

Comment 1

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

https://tbpl.mozilla.org/php/getParsedLog.php?id=51060864&tree=Jetpack&full=1#error0

Comment 2

[Tomislav Jovanovic :zombie]

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

in previous attempt at this, i used sync XHR to delay the loading of a document in order test readyState/contentScriptWhen timings. i'm guessing that (at least some) test VMs have only a single core, so although sync XHR spins the event loop, it still keeps the CPU busy, and attaching the content worker can happen late.

i dug around our test/lib/httpd.js, and found that it does indeed support async http responses (even if we never used it). i think this is a much better solution, as it keeps even a single-core CPU free to process other events during the delay..

Comment 3

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

Comment on attachment 8511680 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1685

Looks good so far!  I mentioned some issues in the pull request that I'd like to see changed for r+ tho.

Comment 4

[Tomislav Jovanovic :zombie]

Comment on attachment 8511680 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1685

addressed review comments except for writing a test for the contentScriptWhenServer. that feels like overkill. this is not shipping code, just tests, and a test _helper_ function at that (i don't see a single example where we test those).

i don't really see the point of testing this helper function to be up to some "specification". i don't need it to behave precisely. all i need from it is to delay the loading of the page enough for events to fire.

and there is no way for this function to behave "improperly" in a way to still pass the test in case of an implementation bug (which is the sole point of tests). the worst case scenario is for it to not return anything, or not delay the response, and both of those cases would just fail the existing tests that use it.

Comment 5

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

Comment on attachment 8511680 [details] [review]
Link to Github pull-request: https://github.com/mozilla/addon-sdk/pull/1685

alright looks good to me, thanks!

Comment 6

[[github robot]]

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

https://github.com/mozilla/addon-sdk/commit/8605c17091be06ca1d6b9fe6f54ae64cc970874c
bug 1089238 - use test/httpd to simulate readyState timings

https://github.com/mozilla/addon-sdk/commit/ff016c3eed2f7e4ee87b31c371627e0fef70a21e
Merge pull request #1685 from zombie/1089238-testContentScriptWhen

bug 1089238 - use test/httpd to simulate readyState timings, r=@erikvold

Comment 7

[Tomislav Jovanovic :zombie]

this looks fixed..