Closed Bug 1089238 Opened 10 years ago Closed 10 years ago

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

Categories

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

defect

Tracking

(firefox34 unaffected, firefox35 unaffected, firefox36 fixed, firefox-esr31 unaffected)

RESOLVED FIXED
mozilla36
Tracking Status
firefox34 --- unaffected
firefox35 --- unaffected
firefox36 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: evold, Assigned: zombie)

References

()

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

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]
Assignee: nobody → tomica+amo
Blocks: 1058698
Status: NEW → ASSIGNED
Priority: -- → P1
OS: Linux → All
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..
Attachment #8511680 - Flags: review?(evold)
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.
Attachment #8511680 - Flags: review?(evold)
Attachment #8511680 - Flags: review-
Attachment #8511680 - Flags: feedback+
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.
Attachment #8511680 - Flags: review- → review?(evold)
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!
Attachment #8511680 - Flags: review?(evold) → review+
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
this looks fixed..
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: