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..0d6d26f7a51b7a6ac581d8714719dfcd35bcba4f
--- /dev/null
+++ b/ash/metrics/user_metrics_recorder_unittest.cc
@@ -0,0 +1,179 @@
+// 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"

[pkotwicz, 2015/04/25 00:16:53]

Nit: You can remove this include?

[bruthig, 2015/04/27 15:22:21]

Done.

+#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_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;
+

[oshima, 2015/04/25 01:06:58]

nit:  // test::AshTestBase:

[bruthig, 2015/04/27 15:22:21]

Done.

+  void SetUp() override;
+  void TearDown() override;
+
+  // Sets the user login status.
+  void SetLoginStatus(user::LoginStatus login_status);
+
+  // Sets the current user session to be (in)active in a multi window

[pkotwicz, 2015/04/25 00:16:53]

Nit: I think that "active or inactive" is clearer than "(in)active"

[bruthig, 2015/04/27 15:22:21]

Done.

+  // environment.
+  void SetUserActiveInMultiWindowEnvironment(bool is_active);
+

[pkotwicz, 2015/04/25 00:16:53]

I don't think that you use this method

[bruthig, 2015/04/27 15:22:21]

Done.

+  UserMetricsRecorder* user_metrics_recorder() {
+    return user_metrics_recorder_.get();
+  }

[oshima, 2015/04/25 01:06:58]

This seems to be unused?

[bruthig, 2015/04/27 15:22:21]

Done.

+
+  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()));

[oshima, 2015/04/25 01:06:58]

what's the benefit of creating recorder, then create test api, if the recorder
itself isn't used?

If you want to access the recorder, it'd be better to let test api create the
recorder internally, then get reference from it when necessary. That can
minimize
exposure of the test only constructor.

[bruthig, 2015/04/27 15:22:21]

Done.

+
+  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(
+    bool is_active) {
+  if (is_active) {
+    SetLoginStatus(user::LOGGED_IN_USER);
+    ASSERT_TRUE(user_metrics_recorder_test_api()
+                    ->IsUserActiveInMultiWindowEnvironment());
+  } else {
+    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 IsUserActive() dependent stats are not recorded when a
+// user is not active.
+TEST_F(UserMetricsRecorderTest, VerifyStatsRecordedWhenUserIsNotActive) {
+  SetUserActiveInMultiWindowEnvironment(false);
+  user_metrics_recorder_test_api()->RecordPeriodicMetrics();
+
+  histograms().ExpectTotalCount(kAsh_NumberOfVisibleWindowsInPrimaryDisplay, 0);
+}
+
+// Verifies that the IsUserActive() dependent stats are recorded when a user
+// is active.
+TEST_F(UserMetricsRecorderTest, VerifyStatsRecordedWhenIsUserActive) {
+  SetUserActiveInMultiWindowEnvironment(true);
+  user_metrics_recorder_test_api()->RecordPeriodicMetrics();
+
+  histograms().ExpectTotalCount(kAsh_NumberOfVisibleWindowsInPrimaryDisplay, 1);
+}
+
+// Verifies recording of stats which are always recorded by
+// RecordPeriodicMetrics.
+TEST_F(UserMetricsRecorderTest, VerifyStatsRecordedByRecordPeriodicMetrics) {
+  SetUserActiveInMultiWindowEnvironment(true);
+  user_metrics_recorder_test_api()->RecordPeriodicMetrics();
+
+  histograms().ExpectTotalCount(kAsh_ActiveWindowShowTypeOverTime, 1);
+}
+
+}  // namespace ash