Skip to content

GitLab

T
tangled
Commit 530f0508 authored by Toby Huang's avatar Toby Huang Committed by Chromium LUCI CQ
Browse files
Options

Metrics: Refactor FamilyUserDeviceMetricsTest to use LoggedInUserMixin 

This CL
* Refactors FamilyUserDeviceMetricsTest to use LoggedInUserMixin,
eliminating a lot of duplicated code that's already encapsulated in
LoggedInUserMixin and shortening the browser test class.
* Breaks monolithic tests into smaller, modularized pieces to make
them easier to understand and debug when they break.
* Removes some unneeded includes, like UserActionTester and
ScopedTestingCrosSettings.

Bug: 1133856
Change-Id: I098a3028dd021379a46fa619e7f597717507e1ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2602701
Commit-Queue: Toby Huang <tobyhuang@chromium.org>
Reviewed-by: default avatarAga Wronska <agawronska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846350}
parent 4f31344b
Show whitespace changes
Inline Side-by-side
Showing with 118 additions and 149 deletions
chrome/browser/chromeos/child_accounts/family_user_device_metrics_browsertest.cc
View file @ 530f0508
...@@ -5,215 +5,184 @@ ...@@ -5,215 +5,184 @@
#include "chrome/browser/chromeos/child_accounts/family_user_device_metrics.h" #include "chrome/browser/chromeos/child_accounts/family_user_device_metrics.h"
#include <tuple> #include <tuple>
#include <vector>
#include "base/optional.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/metrics/user_action_tester.h" #include "chrome/browser/chromeos/login/test/logged_in_user_mixin.h"
#include "base/values.h"
#include "chrome/browser/chromeos/login/test/embedded_test_server_mixin.h"
#include "chrome/browser/chromeos/login/test/fake_gaia_mixin.h"
#include "chrome/browser/chromeos/login/test/local_policy_test_server_mixin.h"
#include "chrome/browser/chromeos/login/test/login_manager_mixin.h" #include "chrome/browser/chromeos/login/test/login_manager_mixin.h"
#include "chrome/browser/chromeos/login/test/user_policy_mixin.h" #include "chrome/browser/chromeos/login/test/user_policy_mixin.h"
#include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h" #include "chrome/browser/chromeos/login/users/fake_chrome_user_manager.h"
#include "chrome/browser/chromeos/login/wizard_controller.h"
#include "chrome/browser/chromeos/settings/scoped_testing_cros_settings.h"
#include "chrome/browser/chromeos/settings/stub_cros_settings_provider.h"
#include "chrome/browser/ui/browser.h"
#include "chromeos/login/auth/user_context.h"
#include "chromeos/settings/cros_settings_names.h"
#include "components/account_id/account_id.h" #include "components/account_id/account_id.h"
#include "components/user_manager/user_manager.h" #include "components/user_manager/user_manager.h"
#include "components/user_manager/user_type.h"
#include "content/public/test/browser_test.h" #include "content/public/test/browser_test.h"
#include "net/dns/mock_host_resolver.h"
namespace chromeos { namespace chromeos {
namespace { namespace {
const LoginManagerMixin::TestUserInfo kChild{
AccountId::FromUserEmailGaiaId("child@gmail.com", "123456780"),
user_manager::USER_TYPE_CHILD};
const LoginManagerMixin::TestUserInfo kRegular{
AccountId::FromUserEmailGaiaId("regular@gmail.com", "123456789"),
user_manager::USER_TYPE_REGULAR};
const AccountId kDefaultOwnerAccountId = const AccountId kDefaultOwnerAccountId =
AccountId::FromUserEmailGaiaId("owner@gmail.com", "123456781"); AccountId::FromUserEmailGaiaId(test::kTestEmail, test::kTestGaiaId);
std::vector<LoginManagerMixin::TestUserInfo> GetInitialUsers(bool with_child) {
if (with_child)
return {kChild};
return {kRegular};
}
} // namespace } // namespace
// Test params: // Test params:
// - is_initial_user_child: whether the user presented in the initial user list // - LogInType: regular or child.
// is child. // - IsUserExisting: if false, no existing users on the login screen.
// - is_initial_user_device_owner: whether the user presented in the initial
// user list is device owner.
class FamilyUserDeviceMetricsTest class FamilyUserDeviceMetricsTest
: public MixinBasedInProcessBrowserTest, : public MixinBasedInProcessBrowserTest,
public testing::WithParamInterface<std::tuple<bool, bool>> { public testing::WithParamInterface<
public: std::tuple<LoggedInUserMixin::LogInType, /*IsUserExisting=*/bool>> {
void SetUpOnMainThread() override { protected:
MixinBasedInProcessBrowserTest::SetUpOnMainThread(); bool IsUserChild() const {
return GetLogInType() == LoggedInUserMixin::LogInType::kChild;
host_resolver()->AddRule("*", "127.0.0.1"); }
fake_gaia_.SetupFakeGaiaForChildUser( bool IsUserExisting() const { return std::get<1>(GetParam()); }
kChild.account_id.GetUserEmail(), kChild.account_id.GetGaiaId(),
FakeGaiaMixin::kFakeRefreshToken, false /*issue_any_scope_token*/);
// Child users require a user policy, set up an empty one so the user can
// get through login.
ASSERT_TRUE(user_policy_mixin_.RequestPolicyUpdate());
WizardController::SkipPostLoginScreensForTesting(); FakeChromeUserManager* user_manager_ = nullptr;
AccountId owner_account = EmptyAccountId(); LoggedInUserMixin logged_in_user_mixin_{
if (!IsInitialUserDeviceOwner()) { &mixin_host_,
owner_account = kDefaultOwnerAccountId; GetLogInType(),
} else if (IsInitialUserChild()) { embedded_test_server(),
owner_account = kChild.account_id; this,
} else { /*should_launch_browser=*/false,
owner_account = kRegular.account_id; /*account_id=*/base::nullopt,
} /*include_initial_user=*/IsUserExisting()};
FakeChromeUserManager* user_manager = private:
// MixinBasedInProcessBrowserTest:
void SetUpOnMainThread() override {
MixinBasedInProcessBrowserTest::SetUpOnMainThread();
// Child users require user policy. Set up an empty one so the user can get
// through login.
user_policy_mixin_.RequestPolicyUpdate();
user_manager_ =
static_cast<FakeChromeUserManager*>(user_manager::UserManager::Get()); static_cast<FakeChromeUserManager*>(user_manager::UserManager::Get());
user_manager->SetOwnerId(owner_account);
scoped_testing_cros_settings_.device_settings()->Set(
kDeviceOwner, base::Value(owner_account.GetUserEmail()));
} }
protected: LoggedInUserMixin::LogInType GetLogInType() const {
void LoginUser(const LoginManagerMixin::TestUserInfo& user_info) { return std::get<0>(GetParam());
UserContext user_context =
LoginManagerMixin::CreateDefaultUserContext(user_info);
user_context.SetRefreshToken(FakeGaiaMixin::kFakeRefreshToken);
login_manager_mixin_.LoginAndWaitForActiveSession(user_context);
} }
bool IsLoggedInUserNew(bool is_logged_in_user_child) const { UserPolicyMixin user_policy_mixin_{&mixin_host_, kDefaultOwnerAccountId};
// The user list always have 1 initial user, either child user or regular };
// user. If the |is_logged_in_user_child| is different from
// IsInitialUserChild(), it indicates that the user type of the initial
// user is different from current logged in user, so the current logged in
// user is new.
return is_logged_in_user_child != IsInitialUserChild();
}
bool IsLoggedInUserDeviceOwner(bool is_logged_in_user_child) const { IN_PROC_BROWSER_TEST_P(FamilyUserDeviceMetricsTest, IsDeviceOwner) {
// If IsInitialUserDeviceOwner() is false, no device owner is set at base::HistogramTester histogram_tester;
// the beginning, otherwise the initial user is the device owner.
if (!IsInitialUserDeviceOwner())
return false;
// Check whether the current logged in user is the initial user. // Set the device owner to the logged in user.
return !IsLoggedInUserNew(is_logged_in_user_child); user_manager_->SetOwnerId(logged_in_user_mixin_.GetAccountId());
} logged_in_user_mixin_.LogInUser();
int GetUserCountOnDevice(bool is_logged_in_user_child) const { histogram_tester.ExpectUniqueSample(
// The user list always have 1 initial user, either child user or regular FamilyUserDeviceMetrics::GetDeviceOwnerHistogramNameForTest(),
// user. If the current logged in user is new, it has 2 users on the device, /*sample=*/true, /*expected_count=*/1);
// otherwise 1. }
return IsLoggedInUserNew(is_logged_in_user_child) ? 2 : 1;
}
bool IsInitialUserChild() const { return std::get<0>(GetParam()); } IN_PROC_BROWSER_TEST_P(FamilyUserDeviceMetricsTest, IsNotDeviceOwner) {
bool IsInitialUserDeviceOwner() const { return std::get<1>(GetParam()); } base::HistogramTester histogram_tester;
private: // Set the device owner to an arbitrary account that's not logged in.
EmbeddedTestServerSetupMixin embedded_test_server_setup_{ user_manager_->SetOwnerId(kDefaultOwnerAccountId);
&mixin_host_, embedded_test_server()}; logged_in_user_mixin_.LogInUser();
FakeGaiaMixin fake_gaia_{&mixin_host_, embedded_test_server()};
LocalPolicyTestServerMixin policy_server_mixin_{&mixin_host_};
UserPolicyMixin user_policy_mixin_{&mixin_host_, kChild.account_id,
&policy_server_mixin_};
LoginManagerMixin login_manager_mixin_{&mixin_host_,
GetInitialUsers(IsInitialUserChild())};
ScopedTestingCrosSettings scoped_testing_cros_settings_;
};
IN_PROC_BROWSER_TEST_P(FamilyUserDeviceMetricsTest, LoginAsChildUser) { histogram_tester.ExpectUniqueSample(
base::UserActionTester user_action_tester; FamilyUserDeviceMetrics::GetDeviceOwnerHistogramNameForTest(),
/*sample=*/false, /*expected_count=*/1);
}
IN_PROC_BROWSER_TEST_P(FamilyUserDeviceMetricsTest, SingleUserAdded) {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
LoginUser(kChild); logged_in_user_mixin_.LogInUser();
// FamilyUserDeviceMetrics::OnNewDay() is triggered in the if (IsUserExisting()) {
// FamilyUserMetricsService constructor, so that metrics // This user has signed into this device before, so they are not new.
// which are reported would have records. histogram_tester.ExpectTotalCount(
histogram_tester.ExpectUniqueSample( FamilyUserDeviceMetrics::GetNewUserAddedHistogramNameForTest(), 0);
FamilyUserDeviceMetrics::GetFamilyLinkUsersCountHistogramNameForTest(), } else {
/*sample=*/1, // This is the first time this user is signing into this device.
/*expected_count=*/1); FamilyUserDeviceMetrics::NewUserAdded sample =
IsUserChild()
? FamilyUserDeviceMetrics::NewUserAdded::kFamilyLinkUserAdded
: FamilyUserDeviceMetrics::NewUserAdded::kRegularUserAdded;
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
FamilyUserDeviceMetrics::GetTotalUsersCountHistogramNameForTest(), FamilyUserDeviceMetrics::GetNewUserAddedHistogramNameForTest(), sample,
/*sample=*/GetUserCountOnDevice(/*is_logged_in_user_child=*/true),
/*expected_count=*/1); /*expected_count=*/1);
}
}
IN_PROC_BROWSER_TEST_P(FamilyUserDeviceMetricsTest, SingleUserCount) {
base::HistogramTester histogram_tester;
logged_in_user_mixin_.LogInUser();
const int family_link_users_count = IsUserChild() ? 1 : 0;
const int total_users_count = 1;
if (IsLoggedInUserNew(/*is_logged_in_user_child=*/true)) {
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
FamilyUserDeviceMetrics::GetNewUserAddedHistogramNameForTest(), FamilyUserDeviceMetrics::GetFamilyLinkUsersCountHistogramNameForTest(),
/*sample=*/FamilyUserDeviceMetrics::NewUserAdded::kFamilyLinkUserAdded, /*sample=*/family_link_users_count,
/*expected_count=*/1); /*expected_count=*/1);
} else {
histogram_tester.ExpectTotalCount(
FamilyUserDeviceMetrics::GetNewUserAddedHistogramNameForTest(), 0);
}
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
FamilyUserDeviceMetrics::GetDeviceOwnerHistogramNameForTest(), FamilyUserDeviceMetrics::GetTotalUsersCountHistogramNameForTest(),
/*sample=*/ /*sample=*/total_users_count,
IsLoggedInUserDeviceOwner(/*is_logged_in_user_child=*/true) ? 1 : 0,
/*expected_count=*/1); /*expected_count=*/1);
} }
IN_PROC_BROWSER_TEST_P(FamilyUserDeviceMetricsTest, LoginAsRegularUser) { IN_PROC_BROWSER_TEST_P(FamilyUserDeviceMetricsTest, LoginAsNewChildUser) {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
LoginUser(kRegular); logged_in_user_mixin_.GetLoginManagerMixin()->LoginAsNewChildUser();
logged_in_user_mixin_.GetLoginManagerMixin()->WaitForActiveSession();
// If existing Family Link user on login screen, then two Family Link users.
const int family_link_users_count = IsUserExisting() && IsUserChild() ? 2 : 1;
// If no existing users on login screen, then this user is the first and only.
const int total_users_count = IsUserExisting() ? 2 : 1;
// FamilyUserDeviceMetrics::OnNewDay() is triggered in the histogram_tester.ExpectUniqueSample(
// FamilyUserMetricsService constructor, so that metrics FamilyUserDeviceMetrics::GetNewUserAddedHistogramNameForTest(),
// which are reported would have records. FamilyUserDeviceMetrics::NewUserAdded::kFamilyLinkUserAdded,
/*expected_count=*/1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
FamilyUserDeviceMetrics::GetFamilyLinkUsersCountHistogramNameForTest(), FamilyUserDeviceMetrics::GetFamilyLinkUsersCountHistogramNameForTest(),
/*sample=*/IsInitialUserChild() ? 1 : 0, /*sample=*/family_link_users_count,
/*expected_count=*/1); /*expected_count=*/1);
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
FamilyUserDeviceMetrics::GetTotalUsersCountHistogramNameForTest(), FamilyUserDeviceMetrics::GetTotalUsersCountHistogramNameForTest(),
/*sample=*/GetUserCountOnDevice(/*is_logged_in_user_child=*/false), /*sample=*/total_users_count,
/*expected_count=*/1); /*expected_count=*/1);
}
IN_PROC_BROWSER_TEST_P(FamilyUserDeviceMetricsTest, LoginAsNewRegularUser) {
base::HistogramTester histogram_tester;
logged_in_user_mixin_.GetLoginManagerMixin()->LoginAsNewRegularUser();
logged_in_user_mixin_.GetLoginManagerMixin()->WaitForActiveSession();
// If existing Family Link user on login screen, then one Family Link user.
const int family_link_users_count = IsUserExisting() && IsUserChild() ? 1 : 0;
// If no existing users on login screen, then this user is the first and only.
const int total_users_count = IsUserExisting() ? 2 : 1;
if (IsLoggedInUserNew(/*is_logged_in_user_child=*/false)) {
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
FamilyUserDeviceMetrics::GetNewUserAddedHistogramNameForTest(), FamilyUserDeviceMetrics::GetNewUserAddedHistogramNameForTest(),
/*sample=*/FamilyUserDeviceMetrics::NewUserAdded::kRegularUserAdded, FamilyUserDeviceMetrics::NewUserAdded::kRegularUserAdded,
/*expected_count=*/1); /*expected_count=*/1);
} else {
histogram_tester.ExpectTotalCount(
FamilyUserDeviceMetrics::GetNewUserAddedHistogramNameForTest(), 0);
}
histogram_tester.ExpectUniqueSample( histogram_tester.ExpectUniqueSample(
FamilyUserDeviceMetrics::GetDeviceOwnerHistogramNameForTest(), FamilyUserDeviceMetrics::GetFamilyLinkUsersCountHistogramNameForTest(),
/*sample=*/ /*sample=*/family_link_users_count,
IsLoggedInUserDeviceOwner(/*is_logged_in_user_child=*/false) ? 1 : 0, /*expected_count=*/1);
histogram_tester.ExpectUniqueSample(
FamilyUserDeviceMetrics::GetTotalUsersCountHistogramNameForTest(),
/*sample=*/total_users_count,
/*expected_count=*/1); /*expected_count=*/1);
} }
INSTANTIATE_TEST_SUITE_P( INSTANTIATE_TEST_SUITE_P(
, ,
FamilyUserDeviceMetricsTest, FamilyUserDeviceMetricsTest,
testing::Combine(/*is_initial_user_child=*/testing::Bool(), testing::Combine(testing::Values(LoggedInUserMixin::LogInType::kChild,
/*is_initial_user_device_owner=*/testing::Bool())); LoggedInUserMixin::LogInType::kRegular),
/*IsUserExisting=*/testing::Bool()));
} // namespace chromeos } // namespace chromeos
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment