New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 704098 link

Starred by 1 user
Status: Available
Owner: ----
Cc:
rockot@google.com
dcheng@chromium.org
█ tibell@chromium.org
sa...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Install dummy ServiceManagerConnection in unit test contexts?

Project Member Reported by blundell@chromium.org, Mar 22 2017 
In unittests the service manager connection is not initialized, leading to code like:

if (ServiceManagerConnection::GetForProcess()) {
...
}

dcheng@ reasonably asks:

Out of curiosity, is there any chance to add some thing like
TestBrowserThreadBundle so that it would Just Work in tests?

(I ask this because I've been surprised by the RenderThreadImpl::current() is
null in tests behavior in the past; it just so happened that the 'null in tests'
check written into non-test code ended up masking other issues)

Comment 1 by roc...@chromium.org, Mar 22 2017 

I added TestServiceManagerContext in content/public/test for essentially this reason. Creating a working ServiceManagerConnection in turn requires a working ServiceManager to live somewhere, and I was converting a "browser test" (which is actually a unit test) that fakes out a ton of the browser environment just to simulate an IPC to an in-process RenderThread. It was do this or delete the test.

Having said that, I am very much not a fan of unit tests that try to fake out too much of the browser environment. We could make TestBrowserThreadBundle instantiate a TestServiceManagerContext if we really need to, but I'd rather avoid it until more than 1 or 2 use cases arise (which is hopefully never.)

Comment 2 by laforge@google.com, Nov 8 2017

Components: Internals>Services>ServiceManager
Bulk applying component Internals>Services>ServiceManager to issues referencing the text ServiceManager.  This may not be 100% accurate, so please feel free to pull the component as needed.

Comment 3 by roc...@chromium.org, Oct 15

Labels: ServiceManagerImprovification
Kind of old now, but I will tag it with the do-something-about-this-soon label since it's relevant to cleaning up SM integration.

In general, I would like to see us kill off ServiceManagerConnection::GetForProcess() altogether.

Comment 4 by roc...@chromium.org, Oct 17

Cc: -roc...@chromium.org rockot@google.com
►

Sign in to add a comment