From 6130ce7663d690dd46386623e868a517b8409fb5 Mon Sep 17 00:00:00 2001 From: David Dorwin <ddorwin@chromium.org> Date: Wed, 22 Feb 2023 20:58:52 +0000 Subject: [PATCH] Reland "[fuchsia] Switch tests to use fake-build-info & enable tests" This is a re-land of commit 15e9ba8ad8a70c4fc9b3a14e91a2117e19870fec / reverts commit 57ff262b2530c7d5142c392419ba421d5186bbea. Reason for re-land: https://fxbug.dev/121700 addresses the issues that led to https://crbug.com/1410009 by using a different minimum shard for system tests that need to run on devices without access to the `fake-build-info` package. Also update the documentation for running gtests on Core as the `fake-build-info` package must be added. Original change's description: > [fuchsia] Switch tests to use fake-build-info & enable tests > > This reverts commit e68789b8185dfd8cac07e439a44b6bfb638f3ee4. > > Reason for revert: https://fxrev.dev/790735 fixes the issue > that led to https://crbug.com/1326322. > > Also remove routing that was only needed by the real build-info service. > > Original change's description: > > [Fuchsia] Remove dependency on fake-build-info > > > > Reverted last change in minimum.shard.test-cml to make it use > > real build-info-service instead of the fake. > > Also disabled BuildInfo test. > > > > Bug: 1326322 > > Change-Id: I04d70a43dd40da6a68a9ec2ddec8e4fbc6dfb881 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3652938 > > Commit-Queue: Sergey Ulanov <sergeyu@chromium.org> > > Reviewed-by: David Dorwin <ddorwin@chromium.org> > > Cr-Commit-Position: refs/heads/main@{#1004539} > > Bug: 1326322, 1326674 > Test: Tests pass on Workstation and Terminal > Change-Id: Ia09c4fa922da87db3259abb325c469c5f2695723 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4183181 > Reviewed-by: Wez <wez@chromium.org> > Commit-Queue: David Dorwin <ddorwin@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1095887} Bug: 1326322, 1326674 Test: base_unittests pass with all build-info references removed from Fuchsia's //src/sys/test_manager/meta/chromium_test_realm.shard.cml Change-Id: I075a3ae40ee1d16494e90907732f264fca80c79d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4240690 Reviewed-by: Wez <wez@chromium.org> Commit-Queue: David Dorwin <ddorwin@chromium.org> Cr-Commit-Position: refs/heads/main@{#1108560} --- base/fuchsia/system_build_info_unittest.cc | 10 ++-------- build/config/fuchsia/test/minimum.shard.test-cml | 11 +++-------- docs/fuchsia/gtests.md | 11 +++++++++-- 3 files changed, 14 insertions(+), 18 deletions(-) diff --git a/base/fuchsia/system_build_info_unittest.cc b/base/fuchsia/system_build_info_unittest.cc index dd7c487f824a1c..15244b7fc5bd4a 100644 --- a/base/fuchsia/system_build_info_unittest.cc +++ b/base/fuchsia/system_build_info_unittest.cc @@ -11,11 +11,7 @@ namespace base { // Ensures that when FetchAndCacheSystemInfo() has not been called in the // process that a DCHECK fires to alert the developer. -// -// TODO(crbug.com/1326674) Ensure the test passes on Fuchsia bots and -// re-enable. -TEST(BuildInfoDeathTest, - DISABLED_GetCachedBuildInfo_DcheckIfNotAlreadyFetched) { +TEST(BuildInfoDeathTest, GetCachedBuildInfo_DcheckIfNotAlreadyFetched) { // Clear the cached build info to force an error condition. ClearCachedSystemInfoForTesting(); @@ -29,9 +25,7 @@ TEST(BuildInfoDeathTest, EXPECT_TRUE(FetchAndCacheSystemInfo()); } -// TODO(crbug.com/1326674) Ensure the test passes on Fuchsia bots and -// re-enable. -TEST(BuildInfoTest, DISABLED_GetCachedBuildInfo_CheckExpectedValues) { +TEST(BuildInfoTest, GetCachedBuildInfo_CheckExpectedValues) { // Ensure the cached BuildInfo is in a known state. ClearCachedSystemInfoForTesting(); ASSERT_TRUE(FetchAndCacheSystemInfo()); diff --git a/build/config/fuchsia/test/minimum.shard.test-cml b/build/config/fuchsia/test/minimum.shard.test-cml index 23f0726cc2d0cd..e18c0fbf9de2c8 100644 --- a/build/config/fuchsia/test/minimum.shard.test-cml +++ b/build/config/fuchsia/test/minimum.shard.test-cml @@ -9,7 +9,7 @@ children: [ { name: "build-info-service", - url: "fuchsia-pkg://fuchsia.com/build-info-service#meta/build-info.cm", + url: "fuchsia-pkg://fuchsia.com/fake-build-info#meta/fake_build_info.cm", }, { name: "intl_property_manager", @@ -17,15 +17,10 @@ }, ], offer: [ - { - directory: "build-info", - from: "parent", - to: "#build-info-service", - }, { protocol: "fuchsia.logger.LogSink", from: "parent", - to: [ "#build-info-service", "#intl_property_manager" ], + to: [ "#intl_property_manager" ], } ], use: [ @@ -76,7 +71,7 @@ facets: { "fuchsia.test": { "deprecated-allowed-packages": [ - "build-info-service", + "fake-build-info", "intl_property_manager", ], }, diff --git a/docs/fuchsia/gtests.md b/docs/fuchsia/gtests.md index e517395ac6d7c9..79eb7f31ce8584 100644 --- a/docs/fuchsia/gtests.md +++ b/docs/fuchsia/gtests.md @@ -153,6 +153,13 @@ and need to be explicitly added as described in At a minimum, `test_manager` is required in order to run tests. This is already available when using a combined repo as in option (1). +<!-- TODO(crbug.com/1408597): Remove this paragraph when the packages for fakes + are subpackaged with the tests. +--> +Another required package is +`//src/developer/build_info/testing:fake-build-info`, which is not available in +Core by default. + Other useful packages include: * `intl_property_manager` - avoids many warnings when the test shard tries to launch it. @@ -185,7 +192,7 @@ The following makes all [additional required packages](#additional-required-packages) available to the Chromium tests. ```bash -$ fx set core.qemu-x64 --auto-dir --release --with //src/testing/fidl/intl_property_manager --with //src/media/audio/audio_core +$ fx set core.qemu-x64 --auto-dir --release --with //src/developer/build_info/testing:fake-build-info --with //src/testing/fidl/intl_property_manager --with //src/media/audio/audio_core ``` ##### Chromium-only repo: packages in base @@ -211,7 +218,7 @@ equivalent to the command line in [Combined repo](#combined-repo). It adds all It also adds `//sdk/lib/sys/cpp` to base packages to enable `base_unittests`'s `TestComponentContextForProcessTest.ProvideSystemService` test to pass. ```bash -$ fx set core.qemu-x64 --auto-dir --release --with-base //src/sys/test_manager --with-base //src/testing/fidl/intl_property_manager --with-base //src/media/audio/audio_core --with-base //sdk/lib/sys/cpp +$ fx set core.qemu-x64 --auto-dir --release --with-base //src/sys/test_manager --with-base //src/developer/build_info/testing:fake-build-info --with-base //src/testing/fidl/intl_property_manager --with-base //src/media/audio/audio_core --with-base //sdk/lib/sys/cpp ``` ### Other Products