Skip to content

Commit

Permalink
Reland "[fuchsia] Switch tests to use fake-build-info & enable tests"
Browse files Browse the repository at this point in the history
This is a re-land of commit 15e9ba8 /
reverts commit 57ff262.

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 e68789b.
>
> 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 <[email protected]>
> > Reviewed-by: David Dorwin <[email protected]>
> > 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 <[email protected]>
> Commit-Queue: David Dorwin <[email protected]>
> 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 <[email protected]>
Commit-Queue: David Dorwin <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1108560}
  • Loading branch information
ddorwin authored and Chromium LUCI CQ committed Feb 22, 2023
1 parent 72262bb commit 6130ce7
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 18 deletions.
10 changes: 2 additions & 8 deletions base/fuchsia/system_build_info_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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());
Expand Down
11 changes: 3 additions & 8 deletions build/config/fuchsia/test/minimum.shard.test-cml
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,18 @@
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",
url: "fuchsia-pkg://fuchsia.com/intl_property_manager#meta/intl_property_manager.cm",
},
],
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: [
Expand Down Expand Up @@ -76,7 +71,7 @@
facets: {
"fuchsia.test": {
"deprecated-allowed-packages": [
"build-info-service",
"fake-build-info",
"intl_property_manager",
],
},
Expand Down
11 changes: 9 additions & 2 deletions docs/fuchsia/gtests.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 6130ce7

Please sign in to comment.