Changed when the UMA metric Ash.NumberOfVisibleWindowsInPrimaryDisplay is recorded.

ash/metrics/user_metrics_recorder_unittest.cc

Index: ash/metrics/user_metrics_recorder_unittest.cc
diff --git a/ash/metrics/user_metrics_recorder_unittest.cc b/ash/metrics/user_metrics_recorder_unittest.cc
new file mode 100644
index 0000000000000000000000000000000000000000..422aab589277dc5672872f1bde60cfba57c127d5
--- /dev/null
+++ b/ash/metrics/user_metrics_recorder_unittest.cc
@@ -0,0 +1,196 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "ash/metrics/user_metrics_recorder.h"
+
+#include "ash/shelf/shelf_layout_manager.h"
+#include "ash/shell.h"
+#include "ash/system/user/login_status.h"
+#include "ash/test/ash_test_base.h"
+#include "ash/test/test_system_tray_delegate.h"
+#include "ash/test/user_metrics_recorder_test_api.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/test/histogram_tester.h"
+#include "ui/aura/window.h"
+
+namespace ash {
+namespace {
+
+const char kAsh_ShelfAlignmentOverTime[] = "Ash.ShelfAlignmentOverTime";
+
+const char kAsh_NumberOfVisibleWindowsInPrimaryDisplay[] =
+    "Ash.NumberOfVisibleWindowsInPrimaryDisplay";
+
+const char kAsh_ActiveWindowShowTypeOverTime[] =
+    "Ash.ActiveWindowShowTypeOverTime";
+
+}  // namespace
+
+// Test fixture for the UserMetricsRecorder class.
+class UserMetricsRecorderTest : public test::AshTestBase {
+ public:
+  UserMetricsRecorderTest();
+  ~UserMetricsRecorderTest() override;
+
+  void SetUp() override;
+  void TearDown() override;
+
+  // Sets the user login status.
+  void SetLoginStatus(user::LoginStatus login_status);
+
+  // Sets the current user session to be active in a multi window environment.
+  void SetUserActiveInMultiWindowEnvironment();
+
+  // Sets the current user session to be inactive in a multi window environment.
+  void SetUserNotActiveInMultiWindowEnvironment();
+
+  UserMetricsRecorder* user_metrics_recorder() {
+    return user_metrics_recorder_.get();
+  }
+
+  test::UserMetricsRecorderTestAPI* user_metrics_recorder_test_api() {
+    return user_metrics_recorder_test_api_.get();
+  }
+
+  base::HistogramTester& histograms() { return histograms_; }
+
+ private:
+  // The test target.
+  scoped_ptr<UserMetricsRecorder> user_metrics_recorder_;
+
+  // Test API to access private members of the test target.
+  scoped_ptr<test::UserMetricsRecorderTestAPI> user_metrics_recorder_test_api_;
+
+  // Histogram value verifier.
+  base::HistogramTester histograms_;
+
+  // The active SystemTrayDelegate. Not owned.
+  test::TestSystemTrayDelegate* test_system_tray_delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(UserMetricsRecorderTest);
+};
+
+UserMetricsRecorderTest::UserMetricsRecorderTest() {
+}
+
+UserMetricsRecorderTest::~UserMetricsRecorderTest() {
+}
+
+void UserMetricsRecorderTest::SetUp() {
+  test::AshTestBase::SetUp();
+
+  user_metrics_recorder_.reset(
+      test::UserMetricsRecorderTestAPI::CreateUserMetricsRecorder(false)
+          .release());
+
+  user_metrics_recorder_test_api_.reset(
+      new test::UserMetricsRecorderTestAPI(user_metrics_recorder_.get()));
+
+  test_system_tray_delegate_ = GetSystemTrayDelegate();
+}
+
+void UserMetricsRecorderTest::TearDown() {
+  test_system_tray_delegate_ = nullptr;
+
+  // Make sure |user_metrics_recorder_test_api_| doesn't access
+  // |user_metrics_recorder_| after it's destructed.
+  user_metrics_recorder_test_api_.reset();
+  user_metrics_recorder_.reset();
+
+  test::AshTestBase::TearDown();
+}
+
+void UserMetricsRecorderTest::SetLoginStatus(user::LoginStatus login_status) {
+  test_system_tray_delegate_->SetLoginStatus(login_status);
+}
+
+void UserMetricsRecorderTest::SetUserActiveInMultiWindowEnvironment() {

[pkotwicz, 2015/04/21 16:15:17]

Nit: I would make the method take a boolean.

[bruthig, 2015/04/24 20:00:11]

Done.

+  SetLoginStatus(user::LOGGED_IN_USER);
+  ASSERT_TRUE(
+      user_metrics_recorder_test_api()->IsUserActiveInMultiWindowEnvironment());
+}
+
+void UserMetricsRecorderTest::SetUserNotActiveInMultiWindowEnvironment() {
+  SetLoginStatus(user::LOGGED_IN_LOCKED);
+  ASSERT_FALSE(
+      user_metrics_recorder_test_api()->IsUserActiveInMultiWindowEnvironment());
+}
+
+TEST_F(UserMetricsRecorderTest,
+       VerifyIsUserActiveInMultiWindowEnvironmentValues) {
+  SetLoginStatus(user::LOGGED_IN_NONE);
+  EXPECT_FALSE(
+      user_metrics_recorder_test_api()->IsUserActiveInMultiWindowEnvironment());
+
+  SetLoginStatus(user::LOGGED_IN_LOCKED);
+  EXPECT_FALSE(
+      user_metrics_recorder_test_api()->IsUserActiveInMultiWindowEnvironment());
+
+  SetLoginStatus(user::LOGGED_IN_USER);
+  EXPECT_TRUE(
+      user_metrics_recorder_test_api()->IsUserActiveInMultiWindowEnvironment());
+
+  SetLoginStatus(user::LOGGED_IN_OWNER);
+  EXPECT_TRUE(
+      user_metrics_recorder_test_api()->IsUserActiveInMultiWindowEnvironment());
+
+  SetLoginStatus(user::LOGGED_IN_GUEST);
+  EXPECT_TRUE(
+      user_metrics_recorder_test_api()->IsUserActiveInMultiWindowEnvironment());
+
+  SetLoginStatus(user::LOGGED_IN_PUBLIC);
+  EXPECT_TRUE(
+      user_metrics_recorder_test_api()->IsUserActiveInMultiWindowEnvironment());
+
+  SetLoginStatus(user::LOGGED_IN_SUPERVISED);
+  EXPECT_TRUE(
+      user_metrics_recorder_test_api()->IsUserActiveInMultiWindowEnvironment());
+
+  SetLoginStatus(user::LOGGED_IN_KIOSK_APP);
+  EXPECT_FALSE(
+      user_metrics_recorder_test_api()->IsUserActiveInMultiWindowEnvironment());
+}
+
+// Verifies that the stats dependent on the presence of a ShelfLayoutManager
+// are recorded when a ShelfLayoutManager exists.
+// TODO(bruthig): Add a test to verify the stats are not recorded when a
+// ShelfLayoutManager does not exist. Consider investigating whether the
+// conditional is necessary.

[pkotwicz, 2015/04/21 16:15:17]

Remove the test given that we are unsure whether the current behavior is
correct.

[bruthig, 2015/04/24 20:00:11]

Done.

+TEST_F(UserMetricsRecorderTest,
+       VerifyStatsRecordedWhenAShelfLayoutManagerExists) {
+  SetUserActiveInMultiWindowEnvironment();
+  ASSERT_TRUE(ShelfLayoutManager::ForShelf(Shell::GetPrimaryRootWindow()));
+  user_metrics_recorder_test_api()->RecordPeriodicMetrics();
+
+  histograms().ExpectTotalCount(kAsh_ShelfAlignmentOverTime, 1);
+}
+
+// Verifies that the IsUserActive() dependent stats are not recorded when a
+// user is not active.

[pkotwicz, 2015/04/21 16:15:17]

Nit: Please update the test comment

[bruthig, 2015/04/24 20:00:11]

Done.

+TEST_F(UserMetricsRecorderTest, VerifyStatsRecordedWhenUserIsNotActive) {
+  SetUserNotActiveInMultiWindowEnvironment();
+  user_metrics_recorder_test_api()->RecordPeriodicMetrics();
+
+  histograms().ExpectTotalCount(kAsh_NumberOfVisibleWindowsInPrimaryDisplay, 0);
+}
+
+// Verifies that the IsUserActive() dependent stats are recorded when a user
+// is active.

[pkotwicz, 2015/04/21 16:15:17]

Nit: Please update the test comment

[bruthig, 2015/04/24 20:00:12]

Done.

+TEST_F(UserMetricsRecorderTest, VerifyStatsRecordedWhenIsUserActive) {
+  SetUserActiveInMultiWindowEnvironment();
+  user_metrics_recorder_test_api()->RecordPeriodicMetrics();
+
+  histograms().ExpectTotalCount(kAsh_NumberOfVisibleWindowsInPrimaryDisplay, 1);
+}
+
+// Verifies which stats are always recorded and are not dependent on conditional
+// logic for when they are recorded.

[pkotwicz, 2015/04/21 16:15:17]

How about: "Verifies recording of stats which are always recorded."

[bruthig, 2015/04/24 20:00:11]

Done.

+TEST_F(UserMetricsRecorderTest, VerifyStatsRecordedByRecordPeriodicMetrics) {
+  SetUserActiveInMultiWindowEnvironment();
+  user_metrics_recorder_test_api()->RecordPeriodicMetrics();
+
+  histograms().ExpectTotalCount(kAsh_ActiveWindowShowTypeOverTime, 1);
+}
+
+}  // namespace ash