Skip to content

Commit

Permalink
guest_os_share_path: Move SetUpVolume into test SetUp
Browse files Browse the repository at this point in the history
Instead of every test calling SetUpVolume, move it into the test class
SetUp. Also split out the one-off setup from the per-share setup since
that'll be useful in an upcoming change.

Bug: b:240491232
Test: CQ

Change-Id: I017de25421847b700e49a6aba0be9805f146299d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4015663
Commit-Queue: David Munro <davidmunro@google.com>
Reviewed-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1069018}
  • Loading branch information
David Munro authored and Chromium LUCI CQ committed Nov 9, 2022
1 parent 612d34c commit 466a3bd
Showing 1 changed file with 17 additions and 61 deletions.
78 changes: 17 additions & 61 deletions chrome/browser/ash/guest_os/guest_os_share_path_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -228,27 +228,13 @@ class GuestOsSharePathTest : public testing::Test {
ash::ChunneldClient::Shutdown();
}

void SetUpVolume() {
// Setup Downloads and path to share, which depend on MyFilesVolume flag,
// thus can't be on SetUp.
ash::disks::DiskMountManager::InitializeForTesting(
new file_manager::FakeDiskMountManager);
file_manager::VolumeManagerFactory::GetInstance()->SetTestingFactory(
profile(), base::BindRepeating(&BuildVolumeManager));
root_ = file_manager::util::GetMyFilesFolderForProfile(profile());
file_manager::VolumeManager::Get(profile())
->RegisterDownloadsDirectoryForTesting(root_);
share_path_ = root_.Append("path-to-share");
shared_path_ = root_.Append("already-shared");
ASSERT_TRUE(base::CreateDirectory(shared_path_));
void SharePathFor(std::string vm_name) {
ScopedDictPrefUpdate update(profile()->GetPrefs(),
prefs::kGuestOSPathsSharedToVms);
base::Value::List termina;
termina.Append(crostini::kCrostiniDefaultVmName);
update->Set(shared_path_.value(), std::move(termina));
volume_downloads_ = file_manager::Volume::CreateForDownloads(root_);
guest_os_share_path_->RegisterSharedPath(crostini::kCrostiniDefaultVmName,
shared_path_);
base::Value::List pref;
pref.Append(vm_name);
update->Set(shared_path_.value(), std::move(pref));
guest_os_share_path_->RegisterSharedPath(vm_name, shared_path_);
// Run threads now to allow watcher for shared_path_ to start.
task_environment_.RunUntilIdle();
}
Expand Down Expand Up @@ -294,6 +280,18 @@ class GuestOsSharePathTest : public testing::Test {

g_browser_process->platform_part()
->InitializeSchedulerConfigurationManager();
ash::disks::DiskMountManager::InitializeForTesting(
new file_manager::FakeDiskMountManager);
file_manager::VolumeManagerFactory::GetInstance()->SetTestingFactory(
profile(), base::BindRepeating(&BuildVolumeManager));
root_ = file_manager::util::GetMyFilesFolderForProfile(profile());
file_manager::VolumeManager::Get(profile())
->RegisterDownloadsDirectoryForTesting(root_);
share_path_ = root_.Append("path-to-share");
shared_path_ = root_.Append("already-shared");
volume_downloads_ = file_manager::Volume::CreateForDownloads(root_);
ASSERT_TRUE(base::CreateDirectory(shared_path_));
SharePathFor(crostini::kCrostiniDefaultVmName);
}

void TearDown() override {
Expand Down Expand Up @@ -345,7 +343,6 @@ class GuestOsSharePathTest : public testing::Test {
};

TEST_F(GuestOsSharePathTest, SuccessMyFilesRoot) {
SetUpVolume();
base::FilePath my_files =
file_manager::util::GetMyFilesFolderForProfile(profile());
guest_os_share_path_->SharePath(
Expand All @@ -359,7 +356,6 @@ TEST_F(GuestOsSharePathTest, SuccessMyFilesRoot) {
}

TEST_F(GuestOsSharePathTest, SuccessNoPersist) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, share_path_,
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -371,7 +367,6 @@ TEST_F(GuestOsSharePathTest, SuccessNoPersist) {
}

TEST_F(GuestOsSharePathTest, SuccessPersist) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, share_path_,
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -383,7 +378,6 @@ TEST_F(GuestOsSharePathTest, SuccessPersist) {
}

TEST_F(GuestOsSharePathTest, SuccessDriveFsMyDrive) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, drivefs_.Append("root").Append("my"),
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -395,7 +389,6 @@ TEST_F(GuestOsSharePathTest, SuccessDriveFsMyDrive) {
}

TEST_F(GuestOsSharePathTest, SuccessDriveFsMyDriveRoot) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, drivefs_.Append("root"),
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -407,7 +400,6 @@ TEST_F(GuestOsSharePathTest, SuccessDriveFsMyDriveRoot) {
}

TEST_F(GuestOsSharePathTest, FailDriveFsRoot) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, drivefs_,
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -418,7 +410,6 @@ TEST_F(GuestOsSharePathTest, FailDriveFsRoot) {
}

TEST_F(GuestOsSharePathTest, SuccessDriveFsTeamDrives) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, drivefs_.Append("team_drives").Append("team"),
base::BindOnce(
Expand All @@ -431,7 +422,6 @@ TEST_F(GuestOsSharePathTest, SuccessDriveFsTeamDrives) {

// TODO(crbug.com/917920): Enable when DriveFS enforces allowed write paths.
TEST_F(GuestOsSharePathTest, DISABLED_SuccessDriveFsComputersGrandRoot) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, drivefs_.Append("Computers"),
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -444,7 +434,6 @@ TEST_F(GuestOsSharePathTest, DISABLED_SuccessDriveFsComputersGrandRoot) {

// TODO(crbug.com/917920): Remove when DriveFS enforces allowed write paths.
TEST_F(GuestOsSharePathTest, Bug917920DriveFsComputersGrandRoot) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, drivefs_.Append("Computers"),
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -456,7 +445,6 @@ TEST_F(GuestOsSharePathTest, Bug917920DriveFsComputersGrandRoot) {

// TODO(crbug.com/917920): Enable when DriveFS enforces allowed write paths.
TEST_F(GuestOsSharePathTest, DISABLED_SuccessDriveFsComputerRoot) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, drivefs_.Append("Computers").Append("pc"),
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -469,7 +457,6 @@ TEST_F(GuestOsSharePathTest, DISABLED_SuccessDriveFsComputerRoot) {

// TODO(crbug.com/917920): Remove when DriveFS enforces allowed write paths.
TEST_F(GuestOsSharePathTest, Bug917920DriveFsComputerRoot) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, drivefs_.Append("Computers").Append("pc"),
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -480,7 +467,6 @@ TEST_F(GuestOsSharePathTest, Bug917920DriveFsComputerRoot) {
}

TEST_F(GuestOsSharePathTest, SuccessDriveFsComputersLevel3) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0,
drivefs_.Append("Computers").Append("pc").Append("SyncFolder"),
Expand All @@ -494,7 +480,6 @@ TEST_F(GuestOsSharePathTest, SuccessDriveFsComputersLevel3) {
}

TEST_F(GuestOsSharePathTest, SuccessDriveFsFilesById) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, drivefs_.Append(".files-by-id/1234/shared"),
base::BindOnce(
Expand All @@ -506,7 +491,6 @@ TEST_F(GuestOsSharePathTest, SuccessDriveFsFilesById) {
}

TEST_F(GuestOsSharePathTest, SuccessDriveFsShortcutTargetsById) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0,
drivefs_.Append(".shortcut-targets-by-id/1-abc-xyz/shortcut"),
Expand All @@ -520,7 +504,6 @@ TEST_F(GuestOsSharePathTest, SuccessDriveFsShortcutTargetsById) {
}

TEST_F(GuestOsSharePathTest, FailDriveFsTrash) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, drivefs_.Append(".Trash-1000").Append("in-the-trash"),

Expand All @@ -532,7 +515,6 @@ TEST_F(GuestOsSharePathTest, FailDriveFsTrash) {
}

TEST_F(GuestOsSharePathTest, SuccessRemovable) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, base::FilePath("/media/removable/MyUSB"),
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -544,7 +526,6 @@ TEST_F(GuestOsSharePathTest, SuccessRemovable) {
}

TEST_F(GuestOsSharePathTest, FailRemovableRoot) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, base::FilePath("/media/removable"),
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -555,7 +536,6 @@ TEST_F(GuestOsSharePathTest, FailRemovableRoot) {
}

TEST_F(GuestOsSharePathTest, SuccessSystemFonts) {
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-running", 0, base::FilePath("/usr/share/fonts"),
base::BindOnce(
Expand All @@ -566,7 +546,6 @@ TEST_F(GuestOsSharePathTest, SuccessSystemFonts) {
}

TEST_F(GuestOsSharePathTest, SuccessGuestOs) {
SetUpVolume();
file_manager::VolumeManager::Get(profile())->AddSftpGuestOsVolume(
"name", base::FilePath("/media/fuse/whatever"), base::FilePath("/meh"),
guest_os::VmType::UNKNOWN);
Expand All @@ -583,7 +562,6 @@ TEST_F(GuestOsSharePathTest, SuccessGuestOs) {
TEST_F(GuestOsSharePathTest, SharePathErrorSeneschal) {
features_.InitWithFeatures({features::kCrostini}, {});
GetFakeUserManager()->LoginUser(account_id_);
SetUpVolume();
vm_tools::concierge::StartVmResponse start_vm_response;
start_vm_response.set_status(vm_tools::concierge::VM_STATUS_RUNNING);
start_vm_response.mutable_vm_info()->set_seneschal_server_handle(123);
Expand All @@ -605,7 +583,6 @@ TEST_F(GuestOsSharePathTest, SharePathErrorSeneschal) {
}

TEST_F(GuestOsSharePathTest, SharePathErrorPathNotAbsolute) {
SetUpVolume();
const base::FilePath path("not/absolute/dir");
guest_os_share_path_->SharePath(
"vm-running", 0, path,
Expand All @@ -617,7 +594,6 @@ TEST_F(GuestOsSharePathTest, SharePathErrorPathNotAbsolute) {
}

TEST_F(GuestOsSharePathTest, SharePathErrorReferencesParent) {
SetUpVolume();
const base::FilePath path("/path/../references/parent");
guest_os_share_path_->SharePath(
"vm-running", 0, path,
Expand All @@ -629,7 +605,6 @@ TEST_F(GuestOsSharePathTest, SharePathErrorReferencesParent) {
}

TEST_F(GuestOsSharePathTest, SharePathErrorNotUnderDownloads) {
SetUpVolume();
const base::FilePath path("/not/under/downloads");
guest_os_share_path_->SharePath(
"vm-running", 0, path,
Expand All @@ -643,7 +618,6 @@ TEST_F(GuestOsSharePathTest, SharePathErrorNotUnderDownloads) {
TEST_F(GuestOsSharePathTest, SharePathVmToBeRestarted) {
features_.InitWithFeatures({features::kCrostini}, {});
GetFakeUserManager()->LoginUser(account_id_);
SetUpVolume();
guest_os_share_path_->SharePath(
"vm-to-be-started", 0, share_path_,
base::BindOnce(&GuestOsSharePathTest::SharePathCallback,
Expand All @@ -655,7 +629,6 @@ TEST_F(GuestOsSharePathTest, SharePathVmToBeRestarted) {
}

TEST_F(GuestOsSharePathTest, SharePersistedPaths) {
SetUpVolume();
base::FilePath share_path2_ = root_.AppendASCII("path-to-share-2");
ASSERT_TRUE(base::CreateDirectory(share_path2_));
base::Value shared_paths(base::Value::Type::DICTIONARY);
Expand All @@ -675,7 +648,6 @@ TEST_F(GuestOsSharePathTest, SharePersistedPaths) {

TEST_F(GuestOsSharePathTest, RegisterPersistedPaths) {
base::Value shared_paths(base::Value::Type::DICTIONARY);
SetUpVolume();
profile()->GetPrefs()->Set(prefs::kGuestOSPathsSharedToVms, shared_paths);

guest_os_share_path_->RegisterPersistedPaths("v1",
Expand Down Expand Up @@ -747,7 +719,6 @@ TEST_F(GuestOsSharePathTest, RegisterPersistedPaths) {
}

TEST_F(GuestOsSharePathTest, UnsharePathSuccess) {
SetUpVolume();
ScopedDictPrefUpdate update(profile()->GetPrefs(),
prefs::kGuestOSPathsSharedToVms);
base::Value::List vms;
Expand All @@ -763,7 +734,6 @@ TEST_F(GuestOsSharePathTest, UnsharePathSuccess) {
}

TEST_F(GuestOsSharePathTest, UnsharePathRoot) {
SetUpVolume();
guest_os_share_path_->UnsharePath(
"vm-running", root_, true,
base::BindOnce(&GuestOsSharePathTest::UnsharePathCallback,
Expand All @@ -773,7 +743,6 @@ TEST_F(GuestOsSharePathTest, UnsharePathRoot) {
}

TEST_F(GuestOsSharePathTest, UnsharePathVmNotRunning) {
SetUpVolume();
ScopedDictPrefUpdate update(profile()->GetPrefs(),
prefs::kGuestOSPathsSharedToVms);
base::Value::List vms;
Expand All @@ -789,7 +758,6 @@ TEST_F(GuestOsSharePathTest, UnsharePathVmNotRunning) {
}

TEST_F(GuestOsSharePathTest, UnsharePathInvalidPath) {
SetUpVolume();
base::FilePath invalid("invalid/path");
guest_os_share_path_->UnsharePath(
"vm-running", invalid, true,
Expand All @@ -801,7 +769,6 @@ TEST_F(GuestOsSharePathTest, UnsharePathInvalidPath) {
}

TEST_F(GuestOsSharePathTest, GetPersistedSharedPaths) {
SetUpVolume();
// path1:['vm1'], path2:['vm2'], path3:['vm3'], path12:['vm1','vm2']
base::Value shared_paths(base::Value::Type::DICTIONARY);

Expand Down Expand Up @@ -846,7 +813,6 @@ TEST_F(GuestOsSharePathTest, GetPersistedSharedPaths) {
}

TEST_F(GuestOsSharePathTest, ShareOnMountSuccessParentMount) {
SetUpVolume();
guest_os_share_path_->set_seneschal_callback_for_testing(base::BindRepeating(
&GuestOsSharePathTest::SeneschalSharePathCallback, base::Unretained(this),
"share-on-mount", shared_path_, crostini::kCrostiniDefaultVmName,
Expand All @@ -859,7 +825,6 @@ TEST_F(GuestOsSharePathTest, ShareOnMountSuccessParentMount) {
}

TEST_F(GuestOsSharePathTest, ShareOnMountSuccessSelfMount) {
SetUpVolume();
auto volume_shared_path =
file_manager::Volume::CreateForDownloads(shared_path_);
guest_os_share_path_->set_seneschal_callback_for_testing(base::BindRepeating(
Expand All @@ -874,8 +839,6 @@ TEST_F(GuestOsSharePathTest, ShareOnMountSuccessSelfMount) {
}

TEST_F(GuestOsSharePathTest, ShareOnMountVmNotRunning) {
SetUpVolume();

// Our test setup mocks out a running VM called kCrostiniDefaultVmName, since
// the other tests with SetUpVolume also use it. Shut it down first so we can
// test the not running case.
Expand All @@ -895,7 +858,6 @@ TEST_F(GuestOsSharePathTest, ShareOnMountVmNotRunning) {
}

TEST_F(GuestOsSharePathTest, ShareOnMountVolumeUnrelated) {
SetUpVolume();
auto volume_unrelated_ = file_manager::Volume::CreateForDownloads(
base::FilePath("/unrelated/path"));

Expand All @@ -911,7 +873,6 @@ TEST_F(GuestOsSharePathTest, ShareOnMountVolumeUnrelated) {
}

TEST_F(GuestOsSharePathTest, UnshareOnUnmountSuccessParentMount) {
SetUpVolume();
guest_os_share_path_->set_seneschal_callback_for_testing(base::BindRepeating(
&GuestOsSharePathTest::SeneschalUnsharePathCallback,
base::Unretained(this), "unshare-on-unmount", shared_path_, Persist::YES,
Expand All @@ -922,7 +883,6 @@ TEST_F(GuestOsSharePathTest, UnshareOnUnmountSuccessParentMount) {
}

TEST_F(GuestOsSharePathTest, UnshareOnUnmountSuccessSelfMount) {
SetUpVolume();
auto volume_shared_path =
file_manager::Volume::CreateForDownloads(shared_path_);
guest_os_share_path_->set_seneschal_callback_for_testing(base::BindRepeating(
Expand All @@ -935,7 +895,6 @@ TEST_F(GuestOsSharePathTest, UnshareOnUnmountSuccessSelfMount) {
}

TEST_F(GuestOsSharePathTest, UnshareOnDeleteMountExists) {
SetUpVolume();
ASSERT_TRUE(base::DeleteFile(shared_path_));
guest_os_share_path_->set_seneschal_callback_for_testing(base::BindRepeating(
&GuestOsSharePathTest::SeneschalUnsharePathCallback,
Expand All @@ -945,7 +904,6 @@ TEST_F(GuestOsSharePathTest, UnshareOnDeleteMountExists) {
}

TEST_F(GuestOsSharePathTest, UnshareOnDeleteMountRemoved) {
SetUpVolume();
// Rename root_ rather than delete to mimic atomic removal of mount.
base::FilePath renamed =
root_.DirName().Append(root_.BaseName().value() + ".tmp");
Expand All @@ -959,7 +917,6 @@ TEST_F(GuestOsSharePathTest, UnshareOnDeleteMountRemoved) {
}

TEST_F(GuestOsSharePathTest, RegisterPathThenUnshare) {
SetUpVolume();
guest_os_share_path_->RegisterSharedPath(crostini::kCrostiniDefaultVmName,
share_path_);
guest_os_share_path_->UnsharePath(
Expand All @@ -972,7 +929,6 @@ TEST_F(GuestOsSharePathTest, RegisterPathThenUnshare) {
}

TEST_F(GuestOsSharePathTest, IsPathShared) {
SetUpVolume();
// shared_path_ and children paths are shared for 'termina'.
for (auto& path : {shared_path_, shared_path_.Append("a.txt"),
shared_path_.Append("a"), shared_path_.Append("a/b")}) {
Expand Down

0 comments on commit 466a3bd

Please sign in to comment.