Some unit tests in gyp build are not in the gn build |
|||||
Issue description(and potentially vice versa?) For example, in the gyp build, view_targeter_unittest.cc is part of views_unittests, but not so in the gn build.
,
Apr 29 2016
Comparing a gyp release build with:
GYP_DEFINES=component=shared_library disable_nacl=1 win_fastlink=1
to a gn build with:
is_debug = false
is_component_build = true
enable_nacl = false
is_win_fastlink = true
at hash d570b1f16e30edb5b6b7254c9549d1355fe1cd03 I see 27397 invocations of cl.exe in the gyp build and only 26929 in the gn build, a 468 translation unit discrepancy.
This is probably a mixture of build target differences and omitted source files. I'm planning to scrape the data to find where the differences are coming from.
Getting the differences to zero is probably a non-goal, but understanding them seems useful. If you have any information on known differences (benign or otherwise) please share.
,
Apr 29 2016
You're certainly not going to be able to get to zero. Also, checking w/ enable_nacl=false is misleading, since you'll be missing a lot of stuff. That said, I have no doubt that we may be missing things.
,
Apr 30 2016
I knew that I had to set gn to x86 to make them match but then I forgot to actually do it. Now I'm testing with that fixed and with enable_nacl=true. So:
GYP_DEFINES=component=shared_library win_fastlink=1
and:
is_debug = false
is_component_build = true
enable_nacl = true
is_win_fastlink = true
target_cpu="x86"
,
Apr 30 2016
I wouldn't count compile steps, those are definitely going to be different. When we flipped Linux, I compiled the unit tests binaries, did --gtest-list-tests (or something like that, sorted the result, then diffed. This worked well.
,
May 1 2016
Didn't Scott build something to compare diffs of lists of compilation units in gyp and gn a while ago that found some of this?
,
May 2 2016
The paths for generated files are certainly different, but is there reasons to expect the set of checked-in .cc files being built to be different? A quick comparison shows that there appear to be 30 *test?.cc files that are compiled in gyp builds but not gn builds. Another source of diffs is files that are platform specific that have been filtered on gn builds but not gyp builds. These are good differences - more reasons to look forward to dropping gyp.
,
May 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/715326a623d42be037eca4c333d9e746eca03ddb commit 715326a623d42be037eca4c333d9e746eca03ddb Author: brucedawson <brucedawson@chromium.org> Date: Wed May 04 20:47:20 2016 Add missing remoting tests to gn build Some .cc files are missing from the .gn build. Adding a few of them. BUG= 604060 Review-Url: https://codereview.chromium.org/1946373002 Cr-Commit-Position: refs/heads/master@{#391631} [modify] https://crrev.com/715326a623d42be037eca4c333d9e746eca03ddb/remoting/client/BUILD.gn [modify] https://crrev.com/715326a623d42be037eca4c333d9e746eca03ddb/remoting/test/BUILD.gn
,
May 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/737b128f6945bd40959db05e8abec9012076a247 commit 737b128f6945bd40959db05e8abec9012076a247 Author: brucedawson <brucedawson@chromium.org> Date: Wed May 04 20:55:02 2016 Add two missing tests to gn build BUG= 604060 Review-Url: https://codereview.chromium.org/1950183002 Cr-Commit-Position: refs/heads/master@{#391637} [modify] https://crrev.com/737b128f6945bd40959db05e8abec9012076a247/chrome/test/BUILD.gn [modify] https://crrev.com/737b128f6945bd40959db05e8abec9012076a247/content/test/BUILD.gn
,
May 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1cbbf55e024f75c92b765ef001691aebfd3234cc commit 1cbbf55e024f75c92b765ef001691aebfd3234cc Author: brucedawson <brucedawson@chromium.org> Date: Thu May 05 04:39:12 2016 Add three missing test files to gn builds BUG= 604060 Review-Url: https://codereview.chromium.org/1951083002 Cr-Commit-Position: refs/heads/master@{#391755} [modify] https://crrev.com/1cbbf55e024f75c92b765ef001691aebfd3234cc/components/metrics/BUILD.gn [modify] https://crrev.com/1cbbf55e024f75c92b765ef001691aebfd3234cc/components/safe_json/BUILD.gn [modify] https://crrev.com/1cbbf55e024f75c92b765ef001691aebfd3234cc/components/translate/core/browser/BUILD.gn
,
May 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1a86e1e5cef109674814b73cfd606acf6152f2cc commit 1a86e1e5cef109674814b73cfd606acf6152f2cc Author: brucedawson <brucedawson@chromium.org> Date: Thu May 05 05:17:42 2016 Add default manifest for gn nacl64.exe BUG= 604060 Review-Url: https://codereview.chromium.org/1945343002 Cr-Commit-Position: refs/heads/master@{#391763} [modify] https://crrev.com/1a86e1e5cef109674814b73cfd606acf6152f2cc/components/nacl/broker/BUILD.gn
,
May 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a51aab488fc2426706646f5d749e54ce8f979fcd commit a51aab488fc2426706646f5d749e54ce8f979fcd Author: nacl-deps-roller <nacl-deps-roller@chromium.org> Date: Thu May 05 06:27:47 2016 Roll src/native_client/ 756d2979a..7965517f3 (1 commit). https://chromium.googlesource.com/native_client/src/native_client.git/+log/756d2979ac26..7965517f3877 $ git log 756d2979a..7965517f3 --date=short --no-merges --format='%ad %ae %s' 2016-05-04 brucedawson Add default Windows manifest to four .gn executables BUG= 604060 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_dbg_32_ng,linux_nacl_sdk_build TBR=mseaborn@chromium.org Review-Url: https://codereview.chromium.org/1947043003 Cr-Commit-Position: refs/heads/master@{#391771} [modify] https://crrev.com/a51aab488fc2426706646f5d749e54ce8f979fcd/DEPS
,
May 5 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cf9b781660acfac6f0a13be66d868252d77588d4 commit cf9b781660acfac6f0a13be66d868252d77588d4 Author: brucedawson <brucedawson@chromium.org> Date: Thu May 05 15:26:37 2016 Add three missing test files to the .gn builds BUG= 604060 Review-Url: https://codereview.chromium.org/1949173002 Cr-Commit-Position: refs/heads/master@{#391806} [modify] https://crrev.com/cf9b781660acfac6f0a13be66d868252d77588d4/google_apis/BUILD.gn
,
May 6 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5982933b15bf02afd96c9d9deedfc334738a103b commit 5982933b15bf02afd96c9d9deedfc334738a103b Author: brucedawson <brucedawson@chromium.org> Date: Fri May 06 19:03:02 2016 Adding nacl component unit tests BUG= 604060 Review-Url: https://codereview.chromium.org/1949423002 Cr-Commit-Position: refs/heads/master@{#392112} [modify] https://crrev.com/5982933b15bf02afd96c9d9deedfc334738a103b/components/BUILD.gn [modify] https://crrev.com/5982933b15bf02afd96c9d9deedfc334738a103b/components/nacl/browser/BUILD.gn
,
May 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/443e67e3f6f3037c3c97ccc1dc3fa2e501bfdb15 commit 443e67e3f6f3037c3c97ccc1dc3fa2e501bfdb15 Author: brucedawson <brucedawson@chromium.org> Date: Mon May 09 20:02:00 2016 Add two missing base tests to gn build BUG= 604060 Review-Url: https://codereview.chromium.org/1961493002 Cr-Commit-Position: refs/heads/master@{#392405} [modify] https://crrev.com/443e67e3f6f3037c3c97ccc1dc3fa2e501bfdb15/base/BUILD.gn
,
May 13 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/63411b4d29eaf35d1dc9c4e9ea48635bd9f34bcd commit 63411b4d29eaf35d1dc9c4e9ea48635bd9f34bcd Author: thakis <thakis@chromium.org> Date: Fri May 13 16:23:29 2016 mac/gn: Add missing test file to base_unittests. BUG= 604060 Review-Url: https://codereview.chromium.org/1970343003 Cr-Commit-Position: refs/heads/master@{#393541} [modify] https://crrev.com/63411b4d29eaf35d1dc9c4e9ea48635bd9f34bcd/base/BUILD.gn
,
May 13 2016
Similar to https://bugs.chromium.org/p/chromium/issues/detail?id=431177#c74 and 75: thakis@thakis:~/src/chrome/src$ comm -2 -3 <(cd out/Release && ls *tests) <(cd out/gn && ls *tests) shell_client_lib_unittests This is looking good on linux! $ (for t in $(cat tests.txt); do echo $t; diff <(out/gn/$t --gtest_list_tests | grep -v GetParam | sort) <(out/Release/$t --gtest_list_tests | grep -v GetParam | sort); done) > ~/output.txt This looks like a lot of tests are missing though.
,
May 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/94b8620d1d6ed812599213a4e20932b469d66a42 commit 94b8620d1d6ed812599213a4e20932b469d66a42 Author: thakis <thakis@chromium.org> Date: Tue May 17 02:03:19 2016 gn/linux: Add missing test to events_unittests. BUG= 604060 Review-Url: https://codereview.chromium.org/1987553002 Cr-Commit-Position: refs/heads/master@{#394019} [modify] https://crrev.com/94b8620d1d6ed812599213a4e20932b469d66a42/ui/events/BUILD.gn
,
May 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/2a9358cc025343df85f97f55f03d8cdbdc64545b commit 2a9358cc025343df85f97f55f03d8cdbdc64545b Author: Nico Weber <thakis@chromium.org> Date: Tue May 17 00:16:11 2016 gn: Add a missing test. BUG= chromium:604060 Change-Id: I7e9b8c1bd7cb14daa15575cfcfe9ba3e8a1e6d1b Reviewed-on: https://chromium-review.googlesource.com/344701 Tryjob-Request: Nico Weber <thakis@chromium.org> Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: Nico Weber <thakis@chromium.org> Commit-Queue: Jamie Madill <jmadill@chromium.org> [modify] https://crrev.com/2a9358cc025343df85f97f55f03d8cdbdc64545b/src/tests/BUILD.gn
,
May 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ef5de4c4c1074c888f423b1bd31ec9bb714ea8f0 commit ef5de4c4c1074c888f423b1bd31ec9bb714ea8f0 Author: thakis <thakis@chromium.org> Date: Tue May 17 16:51:08 2016 gn/linux: Add missing tests to browser_tests. BUG= 604060 Review-Url: https://codereview.chromium.org/1985903003 Cr-Commit-Position: refs/heads/master@{#394146} [modify] https://crrev.com/ef5de4c4c1074c888f423b1bd31ec9bb714ea8f0/chrome/test/BUILD.gn
,
May 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/68097aeefdaa344b037824ddc72affe011ff23ad commit 68097aeefdaa344b037824ddc72affe011ff23ad Author: geofflang <geofflang@chromium.org> Date: Tue May 17 18:57:55 2016 Roll ANGLE de44d3a..be815a4 https://chromium.googlesource.com/angle/angle.git/+log/de44d3a..be815a4 BUG= 610800 , chromium:604060 TBR=jmadill@chromium.org TEST=bots CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.linux:linux_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/1990443002 Cr-Commit-Position: refs/heads/master@{#394180} [modify] https://crrev.com/68097aeefdaa344b037824ddc72affe011ff23ad/DEPS
,
May 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/60dd666a273eb84d361f0fd5de11d770881b46bc commit 60dd666a273eb84d361f0fd5de11d770881b46bc Author: fmenozzi <fmenozzi@google.com> Date: Wed May 18 00:30:06 2016 gn/mac: Add missing test to media_blink_unittests BUG= 604060 Review-Url: https://codereview.chromium.org/1983313002 Cr-Commit-Position: refs/heads/master@{#394284} [modify] https://crrev.com/60dd666a273eb84d361f0fd5de11d770881b46bc/media/blink/BUILD.gn
,
May 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0889f3a0a4cee816619636ef78df09f22801068e commit 0889f3a0a4cee816619636ef78df09f22801068e Author: thakis <thakis@chromium.org> Date: Wed May 18 03:39:46 2016 mac/gn: Add missing test to media_unittests. https://codereview.chromium.org/1748333003 forgot to add this test to gn. This is the only test missing in media_unittests. BUG= 584119 , 604060 Review-Url: https://codereview.chromium.org/1987923003 Cr-Commit-Position: refs/heads/master@{#394325} [modify] https://crrev.com/0889f3a0a4cee816619636ef78df09f22801068e/media/BUILD.gn
,
May 18 2016
,
May 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/69c290c4783727c88f6e1a3a090f4b75554626ff commit 69c290c4783727c88f6e1a3a090f4b75554626ff Author: thakis <thakis@chromium.org> Date: Wed May 18 14:30:44 2016 gn: Add some missing tests to components_unittests. (It looks like there are more tests missing in components_unittests, but I don't understand why the other missing tests are missing -- from looking at the .gn files they should be there, but they aren't.) BUG= 604060 Review-Url: https://codereview.chromium.org/1987263002 Cr-Commit-Position: refs/heads/master@{#394414} [modify] https://crrev.com/69c290c4783727c88f6e1a3a090f4b75554626ff/components/BUILD.gn
,
May 19 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a456f5bda5a89dbe92a59b96a43c3e8dbf59a77d commit a456f5bda5a89dbe92a59b96a43c3e8dbf59a77d Author: brucedawson <brucedawson@chromium.org> Date: Thu May 19 18:32:54 2016 Adding missing wgl_api_unittest.cc to Windows gn build BUG= 604060 Review-Url: https://codereview.chromium.org/1996083002 Cr-Commit-Position: refs/heads/master@{#394818} [modify] https://crrev.com/a456f5bda5a89dbe92a59b96a43c3e8dbf59a77d/ui/gl/BUILD.gn
,
May 19 2016
Remaining missing test files on Windows that I am aware of: Pending fixes: bwe_test_logging.cc - it appears that this was unconditionally left in .gyp files by accident, fixes for gyp and gn are in crrev.com/1990373002 child_trace_message_filter_unittest.cc - fix is waiting for approval in crrev.com/1988353003 Probably not needing fixes: placeholder_unittest.cc - just an empty test needed for gyp, not needed for gn translate_ui_delegate_unittest.cc - Has conditional in .gn file but not in .gyp file. Was always so. Why? Waiting for a response from https://codereview.chromium.org/1632953002#msg55 That's it.
,
May 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04e9985e9f96be6776d3afd3b383e8df3f7fe83e commit 04e9985e9f96be6776d3afd3b383e8df3f7fe83e Author: brucedawson <brucedawson@chromium.org> Date: Fri May 20 17:17:00 2016 Add missing test file to tracing:unit_tests BUG= 604060 Review-Url: https://codereview.chromium.org/1988353003 Cr-Commit-Position: refs/heads/master@{#395105} [modify] https://crrev.com/04e9985e9f96be6776d3afd3b383e8df3f7fe83e/components/tracing/BUILD.gn
,
May 20 2016
Regarding translate_ui_delegate_unittest.cc, crrev.com/1987073003 added that to gn builds: Description include translate_ui_delegate_unittest to gn build BUG= Committed: https://crrev.com/ccca3fc2b211e33e1390675661fc0c20509be13e Cr-Commit-Position: refs/heads/master@{#394629} So, just waiting for review of crrev.com/199037300 and then all tests are aligned.
,
May 23 2016
`components_unittests --gtest_list_tests` has fairly different output in gyp and gn, see comment 17.
,
May 23 2016
I looked at the differences for components_unittests on Windows and found some tests that were gn only but only one set of tests (on Windows anyway) that are gyp only:
ConstrainedWindowViewsTest.
GrowModalDialogSize
MaximumBrowserDialogSize
MaximumWebContentsDialogSize
ShrinkModalDialogSize
These tests are in constrained_window_views_unittest.cc and the reason I didn't detect this by comparing compile targets is that this source file is built in gn builds, it just isn't built into the tests.
There are two places where "//components/constrained_window" is added to the deps of a build target. I was surprised to find that this causes not just the constrained_window/constrained_window target to build, but also constrained_window/unit_tests. By design?
The reason the tests aren't being compiled in is due to this commented out block in components\BUILD.gn:
# TODO bug 522654 Enable this when the undefined symbol is fixed in
# web_modal such that this links.
#deps += [ "//components/constrained_window:unit_tests" ]
I'll follow up with rouslan@.
There may be similar situations in other test executables - I'll investigate.
,
May 31 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/18d45a5ee99333a05c2100a507dcd1c4be978edd commit 18d45a5ee99333a05c2100a507dcd1c4be978edd Author: thakis <thakis@chromium.org> Date: Tue May 31 19:33:57 2016 gn/linux: Add last missing tests to components_unittests. This seems to just work now. BUG= 604060 ,522654 TBR=rouslan Review-Url: https://codereview.chromium.org/2022233002 Cr-Commit-Position: refs/heads/master@{#396896} [modify] https://crrev.com/18d45a5ee99333a05c2100a507dcd1c4be978edd/components/BUILD.gn
,
May 31 2016
At the moment, everything's that in a gyp test binary is also in a gn test binary (on linux).
,
May 31 2016
(using the test from comment 17) |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dpranke@chromium.org
, Apr 20 2016Components: Build
Labels: -Pri-3 Pri-2