Apparent contradiction in documentation

[Avynn Donaghe]

Hi Chromium-Mojo et al!

With the banning of base::RunUntilIdle I've been having to re-factor a test while (hopefully) using  mojo message interceptors to determine when to return control to the test body, i.e. some event is triggered on the test body and the body needs to wait until the service under test has reacted to the event.

I think that this use case will fit the message interceptors but before I get to my official question please let me know if this approach is incorrect!

That being said I have found what looks like a discrepancy in the mojo documentation and I'm hoping to find some clarity.  In the official docs there is an example of an interceptor where the  `swappedImplForTesting` constructor takes in the receiver for testing, returns a pointer to the original service on `GetForwardingInterface` and the `ScopedSwapImplForTesting` templates on the mojo::Receiver for the service.

Pic for reference,
link to doc

And then in mojo/cpp/test_support/test_utils.h the doc comment appears to contradict the actual documentation.  This version explicitly warns the reader that you should not pass the original service pointer on  `GetForwardingInterface`, instead of `this` a pointer to the original service is passed to the ScopedSwapImplForTesting class, and the ScopedSwapImplForTestingClass templates the implementation class directly.

Pic for reference,

It looks like the docs in the source seem to be more accurate at least based on what the compiler is mad about.  But I want to make sure I'm not missing something and they're both valid somehow.  If they're not both valid I can update the docs!

thanks,
Avynn

[Ken Rockot]

On Thursday, August 29, 2024 at 10:32:03 AM UTC-7 Avynn Donaghe wrote:

Hi Chromium-Mojo et al!

With the banning of base::RunUntilIdle I've been having to re-factor a test while (hopefully) using  mojo message interceptors to determine when to return control to the test body, i.e. some event is triggered on the test body and the body needs to wait until the service under test has reacted to the event.

I think that this use case will fit the message interceptors but before I get to my official question please let me know if this approach is incorrect!

I personally prefer to avoid message interceptors in favor of direct observation of the service implementation where feasible, but yes this is a legitimate use of interceptors.

 

That being said I have found what looks like a discrepancy in the mojo documentation and I'm hoping to find some clarity.  In the official docs there is an example of an interceptor where the  `swappedImplForTesting` constructor takes in the receiver for testing, returns a pointer to the original service on `GetForwardingInterface` and the `ScopedSwapImplForTesting` templates on the mojo::Receiver for the service.

Pic for reference,
link to doc

And then in mojo/cpp/test_support/test_utils.h the doc comment appears to contradict the actual documentation.  This version explicitly warns the reader that you should not pass the original service pointer on  `GetForwardingInterface`, instead of `this` a pointer to the original service is passed to the ScopedSwapImplForTesting class, and the ScopedSwapImplForTestingClass templates the implementation class directly.

Pic for reference,

It looks like the docs in the source seem to be more accurate at least based on what the compiler is mad about.  But I want to make sure I'm not missing something and they're both valid somehow.  If they're not both valid I can update the docs!

It seems like the documentation should be updated to match the class comment, as the latter gives more broadly applicable advice.

In practice you can usually get away with the doc's example though, since the distinction only matters when multiple interceptors are applied to the same receiver at the same time. This is probably pretty rare.

thanks,
Avynn