Unit tests missing after moving them into separate GYP static library

19 views
Skip to first unread message

Lukasz

unread,
Aug 19, 2015, 12:30:21 PM8/19/15
to Chromium-dev
Hello,

Context:

I am in the process of moving browser/platform-agnostic part of chrome/browser/chromeos/drive into components/drive.  Currently sources for unit tests under components/drive (i.e. fake_drive_service_unittest.cc or local_file_reader_unittest.cc) are listed and built via chrome/chrome_tests_unit.gypi.

In crrev.com/1296273002 I want to move listing of drive unit tests into targets defined by components/drive.gypi and components/drive/BUILD.gn.  This is needed partly to avoid duplicate basename detection (another pending CL - crrev.com/1296483003 - introduces/renames components/drive/file_system_util_unittest.cc which has the same basename as chrome/browser/chromeos/drive/file_system_util_unittest.cc - this causes a GN failure [1]), but I think separate targets are desirable in general (some arguments are in the CL description at crrev.com/1296273002).
 
Let me also preemptively note that I understand that in the long-term components/drive tests should build into a separate executable, rather than continuing to piggy-back on top of chrome/chrome_tests_unit.gypi - I think crrev.com/1296273002 moves us in that direction.

Problem:

Introducing components/drive.gypi:drive_unit_tests_source_set static library target and depending on it from chrome/chrome_tests_unit.gypi:unit_tests target makes my unit tests "disappear" from out/Debug/unit_tests executable.

Repro steps:

1. $ GYP_DEFINES="use_goma=1 chromeos=1" gclient sync
2. $ ninja -C out/Debug -j 150 unit_tests
3. $ out/Debug/unit_tests --test_filter=*FakeDriveServiceTest*SetLastModifiedTime_Offline*

Before crrev.com/1296273002, the 3rd repro step runs one of tests from components/drive/fake_drive_service_unittest.cc.

After crrev.com/1296273002, the 3rd repro step doesn't find any tests to run.

Notes / questions:

Introducing a syntax error into components/drive/service/fake_drive_service_unittest.cc makes 2nd repro step fail, so I think I can confidently say that building "unit_tests" target depends on compiling "components/drive/service/fake_drive_service_unittest.cc" source file.

I think long time ago I saw a GYP or GN flag for preventing optimizing away unused functions, but 1) I can't find this flag again and 2) I am not sure if this flag is applicable (because AFAIK unit tests are registered by calling MakeAndRegisterTestInfo when executing initializers of global variables introduced by GTEST_TEST_ macro [2]).

Please help?


Thanks,

Lukasz

Brett Wilson

unread,
Aug 19, 2015, 12:47:43 PM8/19/15
to luk...@chromium.org, Chromium-dev
You can't put unit tests in a static library. When static libraries are linked, only those .o files that have symbols that are otherwise undefined will get pulled in. unit test files generally have no such symbols, so they will not be pulled in by the linker. This doesn't have anything to do with unused functions.

GN has the concept of a source_set which solves this problem by avoiding the intermediate static library. This works the way most people expect static libraries to work. But in GYP you can't make an intermediate target and have the unit tests still work. Some people work around this in various ways by having the target list in a .gypi that gets included, so you can still have separation of the .gyp files.

Brett

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

Torne (Richard Coles)

unread,
Aug 19, 2015, 1:15:14 PM8/19/15
to bre...@chromium.org, luk...@chromium.org, Chromium-dev
Is that actually the case? The linker command lines for chromium binaries surround all the dependent gyp target static libraries with -Wl,--whole-archive, and have done for some time now..

Torne (Richard Coles)

unread,
Aug 19, 2015, 1:20:11 PM8/19/15
to bre...@chromium.org, luk...@chromium.org, Chromium-dev
Oh, I see. I'm too used to android: this only applies to solink, not link (i.e. only to shared libraries, not executables). Pretty much everything on android is a shared library, so I never notice the other case ;)
Reply all
Reply to author
Forward
0 new messages