Palette tool laser prototype.

ash/common/system/chromeos/palette/tools/laser_pointer_unittest.cc

+// Copyright (c) 2016 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/common/system/chromeos/palette/palette_tool_manager.h"
+#include "ash/common/system/chromeos/palette/tools/laser_pointer_mode.h"
+#include "ash/common/system/chromeos/palette/tools/laser_pointer_points.h"
+#include "ash/common/wm_shell.h"
+#include "ash/test/ash_test_base.h"
+#include "ui/events/test/event_generator.h"
+
+namespace ash {
+namespace {
+const double kTestPointsLifetime = 5.0;
+}  // namespace
+
+class LaserPointerModeTestApi {
+ public:
+  LaserPointerModeTestApi(std::unique_ptr<LaserPointerMode> instance) {
+    instance_ = std::move(instance);

[jdufault, 2016/08/17 21:41:08]

Use constructor list whenever possible.

  : instance_(std::move(instance)) {}

[sammiequon, 2016/08/18 00:52:08]

Done.

+  }
+  ~LaserPointerModeTestApi() {}
+
+  void OnEnable() { instance_->OnEnable(); }
+  void OnDisable() { instance_->OnDisable(); }
+
+  const LaserPointerPoints& laser_points() {
+    return instance_->laser_pointer_view_->laser_points_;
+  }
+
+  gfx::Point current_mouse_location() {
+    return instance_->current_mouse_location_;
+  }
+
+ private:
+  std::unique_ptr<LaserPointerMode> instance_;
+  DISALLOW_COPY_AND_ASSIGN(LaserPointerModeTestApi);

[jdufault, 2016/08/17 21:41:08]

newline above this

[sammiequon, 2016/08/18 00:52:08]

Done.

+};
+
+namespace {

[jdufault, 2016/08/17 21:41:08]

Merge the different anonymous namespaces.

[sammiequon, 2016/08/18 00:52:08]

Done.

+class LaserPointerModeTest : public test::AshTestBase {
+ public:
+  LaserPointerModeTest()
+      : laser_pointer_points_(
+            base::TimeDelta::FromSecondsD(kTestPointsLifetime)) {}

[jdufault, 2016/08/17 21:41:08]

Make kTestPointsLifetime an integral type and use FromSeconds(...)

[sammiequon, 2016/08/18 00:52:08]

Done.

+  ~LaserPointerModeTest() override {}
+
+  void SetUp() override {
+    AshTestBase::SetUp();
+    laser_pointer_mode_.reset(new LaserPointerModeTestApi(
+        base::WrapUnique<LaserPointerMode>(new LaserPointerMode(nullptr))));

[jdufault, 2016/08/17 21:41:08]

base::MakeUnique

[sammiequon, 2016/08/18 00:52:08]

Done.

+  }
+
+  void TearDown() override {
+    laser_pointer_mode_.reset();
+    AshTestBase::TearDown();
+  }
+
+ protected:
+  std::unique_ptr<LaserPointerModeTestApi> laser_pointer_mode_;
+  LaserPointerPoints laser_pointer_points_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(LaserPointerModeTest);
+};
+}  // namespace
+
+TEST_F(LaserPointerModeTest, LaserPointerInternalCollection) {
+  // Test to test the laser pointers internal collection is handling receiving

[jdufault, 2016/08/17 21:41:08]

Replace "Test to test" with "Tests that the ...". Also eliminate "is handling"
and just say "handles"

[jdufault, 2016/08/17 21:41:08]

Comment goes above TEST_F function

  // Test description
  TEST_F(Foo, Bar) {
  }

[sammiequon, 2016/08/18 00:52:08]

Done.

[sammiequon, 2016/08/18 00:52:08]

Done.

+  // points and that the functions are returning the expected output.
+  gfx::Point point1(1, 1);
+  gfx::Point point2(2, 2);
+  gfx::Point point3(3, 9);
+  gfx::Point point4(30, 0);
+  gfx::Point point5(0, 40);
+  laser_pointer_points_.AddPoint(point1);
+  laser_pointer_points_.AddPoint(point2);
+  laser_pointer_points_.AddPoint(point3);
+  laser_pointer_points_.AddPoint(point4);
+
+  // Test laser pointer points collection and helper functions.
+  EXPECT_EQ(4, laser_pointer_points_.GetNumberOfPoints());

[jdufault, 2016/08/17 21:41:08]

If we're testing the functionality specifically for LaserPointerPoints, we
should probably pull it out into a new test and just construct the
LaserPointerPoints instance directly.

Making sure the points get forwarded properly is a good test as well. I think
that is covered by the other test though where you use the event generator.

[sammiequon, 2016/08/18 00:52:08]

Like write a LaserPointerPoints test in another files? Can you still add the
TEST_F(LaserPointerModeTest, foo) so they get run in the same batch?

[jdufault, 2016/08/19 01:02:08]

You have the LaserPointerRenderer test right now, I think it could be something
like that in this same file.

+  EXPECT_FALSE(laser_pointer_points_.IsEmpty());
+  EXPECT_EQ(gfx::Rect(1, 0, 29, 9), laser_pointer_points_.GetBoundingBox());
+  EXPECT_EQ(point1, laser_pointer_points_.GetOldest().location);
+  EXPECT_EQ(point4, laser_pointer_points_.GetMostRecent().location);
+
+  laser_pointer_points_.AddPoint(point5);
+  EXPECT_EQ(5, laser_pointer_points_.GetNumberOfPoints());
+  EXPECT_EQ(gfx::Rect(0, 0, 30, 40), laser_pointer_points_.GetBoundingBox());
+
+  laser_pointer_points_.Clear();
+  EXPECT_TRUE(laser_pointer_points_.IsEmpty());
+}
+

[jdufault, 2016/08/17 21:41:08]

You should add a test that verifies points will disappear over time.

[sammiequon, 2016/08/18 00:52:08]

Done.

+TEST_F(LaserPointerModeTest, LaserPointerRenderer) {
+  // Test to ensure the class responsible for drawing the laser pointer receives
+  // points from mouse movements as expected.
+  GetEventGenerator().MoveMouseToInHost(gfx::Point(10, 40));
+  EXPECT_EQ(0, laser_pointer_mode_->laser_points().GetNumberOfPoints());
+  laser_pointer_mode_->OnEnable();
+
+  GetEventGenerator().MoveMouseToInHost(gfx::Point(25, 66));
+  GetEventGenerator().MoveMouseToInHost(gfx::Point(91, 38));
+  GetEventGenerator().MoveMouseToInHost(gfx::Point(34, 58));
+  GetEventGenerator().MoveMouseToInHost(gfx::Point(19, 71));
+
+  EXPECT_EQ(5, laser_pointer_mode_->laser_points().GetNumberOfPoints());
+  EXPECT_EQ(gfx::Rect(10, 38, 81, 33),
+            laser_pointer_mode_->laser_points().GetBoundingBox());
+
+  laser_pointer_mode_->OnDisable();
+  GetEventGenerator().MoveMouseToInHost(gfx::Point(34, 58));
+  GetEventGenerator().MoveMouseToInHost(gfx::Point(19, 71));
+  EXPECT_EQ(0, laser_pointer_mode_->laser_points().GetNumberOfPoints());
+
+  laser_pointer_mode_->OnEnable();
+  EXPECT_EQ(1, laser_pointer_mode_->laser_points().GetNumberOfPoints());
+  EXPECT_EQ(gfx::Point(19, 71),
+            laser_pointer_mode_->laser_points().GetMostRecent().location);
+}
+}  // namespace ash