0

[fuchsia] Fix TestLauncher to use persistent storage for /cache.

Previously tests requiring access to /cache storage had the
isolated-cache-storage feature specified in the test suite's component
manifest. This was propagated to each test-batch process directly,
creating the possibility of concurrently executing batches conflicting
with one another. There is also a risk of the system purging all the
isolated cache directories while a test is using cache, especially if
some other test uses the CacheControl API to manually force a purge.

TestLauncher now splits the persistent data directory passed to each
test batch process to use it both as /data and /cache, thereby avoiding
clashes, or unexpected purge effects.

This workaround may be removed/replaced if the TestLauncher is updated
to launch batches as isolated components, and the component framework
updated to support deterministic cache storage for testing.

Bug: 1176165, 1242170
Change-Id: If763d28078d685bbe495a29f8c55b682077d4dfd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3110415
Auto-Submit: Wez <wez@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Wez <wez@chromium.org>
Cr-Commit-Position: refs/heads/main@{#914653}
This commit is contained in:
Wez
2021-08-24 08:20:31 +00:00
committed by Chromium LUCI CQ
parent 8f71070216
commit e6648d30d0
4 changed files with 33 additions and 21 deletions
base/test/launcher
build/config/fuchsia/test
fuchsia/engine

@ -355,15 +355,16 @@ int LaunchChildTestProcessWithOptions(const CommandLine& command_line,
new_options.spawn_flags = FDIO_SPAWN_CLONE_STDIO | FDIO_SPAWN_CLONE_JOB;
const base::FilePath kDataPath(base::kPersistedDataDirectoryPath);
const base::FilePath kCachePath(base::kPersistedCacheDirectoryPath);
// Clone all namespace entries from the current process, except /data, which
// is overridden below.
// Clone all namespace entries from the current process, except /data and
// /cache, which are overridden below.
fdio_flat_namespace_t* flat_namespace = nullptr;
zx_status_t result = fdio_ns_export_root(&flat_namespace);
ZX_CHECK(ZX_OK == result, result) << "fdio_ns_export_root";
for (size_t i = 0; i < flat_namespace->count; ++i) {
base::FilePath path(flat_namespace->path[i]);
if (path == kDataPath) {
if (path == kDataPath || path == kCachePath) {
result = zx_handle_close(flat_namespace->handle[i]);
ZX_CHECK(ZX_OK == result, result) << "zx_handle_close";
} else {
@ -379,25 +380,36 @@ int LaunchChildTestProcessWithOptions(const CommandLine& command_line,
new_options.job_handle = job_handle.get();
// Give this test its own isolated /data directory by creating a new temporary
// subdirectory under data (/data/test-$PID) and binding that to /data on the
// child process.
// subdirectory under data (/data/test-$PID) and binding paths under that to
// /data and /cache in the child process.
// Persistent data storage is mapped to /cache rather than system-provided
// cache storage, to avoid unexpected purges (see crbug.com/1242170).
CHECK(base::PathExists(kDataPath));
// Create the test subdirectory with a name that is unique to the child test
// process (qualified by parent PID and an autoincrementing test process
// index).
static base::AtomicSequenceNumber child_launch_index;
base::FilePath nested_data_path = kDataPath.AppendASCII(
const base::FilePath child_data_path = kDataPath.AppendASCII(
base::StringPrintf("test-%zu-%d", base::Process::Current().Pid(),
child_launch_index.GetNext()));
CHECK(!base::DirectoryExists(nested_data_path));
CHECK(base::CreateDirectory(nested_data_path));
DCHECK(base::DirectoryExists(nested_data_path));
CHECK(!base::DirectoryExists(child_data_path));
CHECK(base::CreateDirectory(child_data_path));
DCHECK(base::DirectoryExists(child_data_path));
// Bind the new test subdirectory to /data in the child process' namespace.
const base::FilePath test_data_dir(child_data_path.AppendASCII("data"));
CHECK(base::CreateDirectory(test_data_dir));
const base::FilePath test_cache_dir(child_data_path.AppendASCII("cache"));
CHECK(base::CreateDirectory(test_cache_dir));
// Transfer handles to the new directories as /data and /cache in the child
// process' namespace.
new_options.paths_to_transfer.push_back(
{kDataPath,
base::OpenDirectoryHandle(nested_data_path).TakeChannel().release()});
base::OpenDirectoryHandle(test_data_dir).TakeChannel().release()});
new_options.paths_to_transfer.push_back(
{kCachePath,
base::OpenDirectoryHandle(test_cache_dir).TakeChannel().release()});
#endif // defined(OS_FUCHSIA)
#if defined(OS_LINUX) || defined(OS_CHROMEOS)
@ -483,7 +495,7 @@ int LaunchChildTestProcessWithOptions(const CommandLine& command_line,
ZX_CHECK(status == ZX_OK, status);
// Cleanup the data directory.
CHECK(DeletePathRecursively(nested_data_path));
CHECK(DeletePathRecursively(child_data_path));
#elif defined(OS_POSIX)
// It is not possible to waitpid() on any leaked sub-processes of the test
// batch process, since those are not direct children of this process.

@ -719,6 +719,15 @@ TEST_F(TestLauncherTest, TestChildTempDir) {
EXPECT_FALSE(DirectoryExists(task_temp));
}
#if defined(OS_FUCHSIA)
// Verifies that test processes have /data, /cache and /tmp available.
TEST_F(TestLauncherTest, ProvidesDataCacheAndTmpDirs) {
EXPECT_TRUE(base::DirectoryExists(base::FilePath("/data")));
EXPECT_TRUE(base::DirectoryExists(base::FilePath("/cache")));
EXPECT_TRUE(base::DirectoryExists(base::FilePath("/tmp")));
}
#endif // defined(OS_FUCHSIA)
// Unit tests to validate UnitTestLauncherDelegate implementation.
class UnitTestLauncherDelegateTester : public testing::Test {
protected:

@ -1,7 +0,0 @@
{
"sandbox": {
"features": [
"isolated-cache-storage"
]
}
}

@ -555,8 +555,6 @@ test("web_engine_unittests") {
"//third_party/fuchsia-sdk/sdk/pkg/scenic_cpp",
]
additional_manifest_fragments = [
"//build/config/fuchsia/test/access_test_data_dir.test-cmx",
# TODO(crbug.com/1185811): Figure out why jit_capabilities is needed.
"//build/config/fuchsia/test/jit_capabilities.test-cmx",