toolkit/components/satchel/test/test_form_autocomplete_with_list.html ran additional tests after finish() was called

RESOLVED FIXED in mozilla15

Status

()

--
minor
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mats, Assigned: mats)

Tracking

unspecified
mozilla15
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

6 years ago
Fallout from the diagnostic patch in bug 677964.
(Assignee)

Comment 1

6 years ago
Created attachment 628694 [details] [diff] [review]
Move SimpleTest.finish() after the last test is done.  Use info() for log messages that aren't tests.
Attachment #628694 - Flags: review?(philringnalda)
Comment on attachment 628694 [details] [diff] [review]
Move SimpleTest.finish() after the last test is done.  Use info() for log messages that aren't tests.

That test lost me three times before I even got down to the patch.
Attachment #628694 - Flags: review?(philringnalda) → review?(dolske)
Comment on attachment 628694 [details] [diff] [review]
Move SimpleTest.finish() after the last test is done.  Use info() for log messages that aren't tests.

Fine with me if it fixes the problem. But... Are you sure this was the issue your diagnostic caught?

I'd have expected the input event to be fired asynchronously. And so the last bit of the case-400 should run before the input event fires. Is the event actually firing synchronously? I bet Mounir will know, his code and test. :)
Attachment #628694 - Flags: review?(dolske) → review?(mounir)
(In reply to Justin Dolske [:Dolske] from comment #3)
> Comment on attachment 628694 [details] [diff] [review]
> Move SimpleTest.finish() after the last test is done.  Use info() for log
> messages that aren't tests.
> 
> Fine with me if it fixes the problem. But... Are you sure this was the issue
> your diagnostic caught?
> 
> I'd have expected the input event to be fired asynchronously. And so the
> last bit of the case-400 should run before the input event fires. Is the
> event actually firing synchronously? I bet Mounir will know, his code and
> test. :)

This is actually not my code nor my test. That was David Zbarsky's. However, I can have a look.
Comment on attachment 628694 [details] [diff] [review]
Move SimpleTest.finish() after the last test is done.  Use info() for log messages that aren't tests.

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

The best thing to do here to be on the safe side would be:
    case 400:
      // Check that the input event is fired.
      input.addEventListener("input", function(event) {
        input.removeEventListener("input", arguments.callee, false);
        ok(true, "oninput should have been received");
        ok(event.bubbles, "input event should bubble");
        ok(event.cancelable, "input event should be cancelable");

        checkForm("Google");
        SimpleTest.finish();
      }, false);

      doKey("down");
      checkForm("");
      doKey("return");

r=me with that.
Attachment #628694 - Flags: review?(mounir) → review+
(Assignee)

Comment 7

6 years ago
OK, landed with that change:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac3642f73f24
Target Milestone: --- → mozilla15

Comment 8

6 years ago
https://hg.mozilla.org/mozilla-central/rev/ac3642f73f24
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.