Create example test of do_await_remote_message

NEW
Unassigned

Status

4 years ago
a year ago

People

(Reporter: jdm, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [lang=js][good first bug])

(Reporter)

Description

4 years ago
From bug 934066 comment 3:

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #3)
> Comment on attachment 826294 [details] [diff] [review]
> Add a simple parent/child synchronization mechanism for xpcshell tests.
> 
> Review of attachment 826294 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't really know the geolocation test well enough to judge your changes,
> but I'll trust your judgement. The xpcshell API seems fine.
> 
> ::: testing/xpcshell/head.js
> @@ +1213,5 @@
> > +/**
> > + * Execute a given function as soon as a particular cross-process message is received.
> > + * Must be paired with do_send_remote_message or equivalent ProcessMessageManager calls.
> > + */
> > +function do_await_remote_message(name, callback)
> 
> Can you put a simple test for this in testing/xpcshell/example?
> 
> @@ +1249,5 @@
> > +  } else {
> > +    mm = Cc["@mozilla.org/childprocessmessagemanager;1"].getService(Ci.nsISyncMessageSender);
> > +    sender = 'sendAsyncMessage';
> > +  }
> > +  mm[sender](name);
> 
> Doesn't feel like using the intermediate sender variable makes this any
> clearer. I'd probably just call the methods directly in the branches of the
> if.

We want to create a new test in testing/xpcshell/examples that demonstrates the use and correctness of do_await_remote_message. We also want to address the second review comment of the code added in bug 934066.

Comment 1

4 years ago
I would like to work on this bug. I have a build setup and would appreciate it if someone pointed me to the piece of code that I have to demonstrate the use and correctness of.
(Reporter)

Comment 2

4 years ago
(In reply to wrahman0 from comment #1)
> I would like to work on this bug. I have a build setup and would appreciate
> it if someone pointed me to the piece of code that I have to demonstrate the
> use and correctness of.

Great! http://mxr.mozilla.org/mozilla-central/search?string=do_await_remote shows the current single test that uses this function, as well as where it is defined. Does that answer your questions?

Comment 3

4 years ago
Thanks Josh, Ill look into this and let you know if I have any further questions.

Comment 4

4 years ago
Josh, here is what I was thinking: 

How I think the test should be implemented:
- Since the do_await_remote_message simply waits for a message and then runs the callback function, I think I can set the test case like this:

function run_test(){
   setTimeout( do_await_remote_message ( "Test event triggered", success ), 1000);
   flagRaised ? pass : fail;
}

function success () { 
   //Toggle a flag to indicate that this was called 
}

Questions that I have:
- From my understanding, the first argument of do_await_remote_message() is used as a "trigger" to call the second argument ( which is the callback ). But I am not sure how this trigger is activated. How do I set off the remote message? The test case that currently uses this function simply passes a string like this: do_await_remote_message('high_acc_enabled', stop_high_accuracy_watch);
(Reporter)

Comment 5

4 years ago
Oh, the piece you're missing is that the example you were looking at is run inside of another test: http://mxr.mozilla.org/mozilla-central/source/dom/tests/unit/test_geolocation_reset_accuracy_wrap.js#35 You'll note that that's where the corresponding do_send_remote call occurs.

With regards to your test, I don't think we should include a timeout. The harness already has a very long timeout for individual tests, and specific in-test timeouts like the one you're using often cause intermittent failures when run on our testing machines (sometimes the tests just hang for extended periods, then continue execution as normal).

Comment 6

4 years ago
So I added the do_send_remote_message and removed the timeout. Here is what my test looks like:

 const Cc = Components.classes;
 const Ci = Components.interfaces;

 function run_test(){
   do_test_pending();
   do_send_remote_message('Trigger Activated');
   do_await_remote_message('Trigger Activated', do_test_finished);
 }

However, when the test is run, it says:

0:01.50 LOG: Thread-1 INFO (xpcshell/head.js) | test MAIN run_test pending (1)
0:01.50 LOG: Thread-1 INFO (xpcshell/head.js) | test pending (2)
0:01.50 LOG: Thread-1 INFO (xpcshell/head.js) | test pending (3)
0:01.50 LOG: Thread-1 INFO (xpcshell/head.js) | test MAIN run_test finished (3)

0:01.50 LOG: Thread-1 INFO running event loop
...
Timeout

I did however see that inside test_geolocation_reset_accuracy_wrap.js, the do_send_remote_message is put inside of a provider. Is that the piece I am missing?

Comment 7

4 years ago
This is taking longer than I expected. However, the good news is that its starting to make sense. I am expecting that by the end of this week, (Sunday) I will have a patch up for review.
(Reporter)

Comment 8

4 years ago
I'm so sorry, I intended to respond to the previous comment and it got lost. Please do check the "Need more information from" box if you're asking a particular person a question, as it generates extra notifications that don't disappear.

I would not expect do_send_remote_message and do_await_remote_message to work in the same test; you need to run one in another test that gets executed via do_run_test_in_child - this starts another process, which allows the communication to happen as expected. That's why there's a test_geolocation_reset_accuracy.js and test_geolocation_reset_accuracy_wrap.js. Does that make sense?

Comment 9

4 years ago
Hey Josh, yeah I didn't realize that I had to check the "Need more info" section. Anyway, what you said about having the wrapper test file makes total sense. I also found some great docs on MDN that should help.

I have been working on another patch today and got sort of caught up in it. I will try to have something up quickly though.

Comment 10

3 years ago
Hello, I would like to become part of this test.  Please let me knkow what I can do and how to get started.  Thanks

Comment 11

a year ago
Hiiii,I would like to work on this.can you guide me and tell me how to get started.
(Reporter)

Updated

a year ago
Mentor: josh
You need to log in before you can comment on or make changes to this bug.