Add a row in the network tray to inform users to turn Bluetooth on to enable Tether.

ash/system/tray/tray_info_label_unittest.cc

+// Copyright 2017 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/system/tray/tray_info_label.h"
+#include "ash/test/ash_test_base.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace ash {
+
+namespace {
+
+class TestEvent : public ui::Event {

[Kyle Horimoto, 2017/07/12 22:45:59]

TestEvent --> TestClickEvent

[lesliewatkins, 2017/07/13 22:12:49]

Done.

+ public:
+  TestEvent() : ui::Event(ui::ET_MOUSE_PRESSED, base::TimeTicks(), 0) {}
+};
+
+class TestNonClickableDelegate : public TrayInfoLabel::Delegate {

[Kyle Horimoto, 2017/07/12 22:45:59]

Just make one TestDelegate class which can be set to clickable or non-clickable:

class TestDelegate : public TrayInfoLabel::Delegate {
 public:
  TestDelegate() {}
  ~TestDelegate() override {}

  void clicked_message_ids() { return clicked_message_ids_; }

  void AddClickableMessageId(int message_id) {
    clickable_message_ids_.insert(message_id);
  }

  // TrayInfoLabel::Delegate:
  void OnLabelClicked(int message_id) override {
    clicked_message_ids_.push_back(message_id);
  }

  void IsLabelClickable(int message_id) override {
    return clickable_message_ids_.find(message_id) !=
clickable_message_ids_.end();
  }

 private:
  std::vector<int> clicked_message_ids_;
  unordered_set<int> clickable_message_ids_;
};

Then configure the TestDelegate how you need to for each test.

[lesliewatkins, 2017/07/13 22:12:49]

Done.

+ public:
+  void OnLabelClicked(int message_id) override {}
+
+  bool LabelIsClickable(int message_id) override { return false; }
+};
+
+class TestClickableDelegate : public TrayInfoLabel::Delegate {
+ public:
+  bool LabelWasClicked() { return was_clicked_; }
+
+  void OnLabelClicked(int message_id) override { was_clicked_ = true; }
+
+  bool LabelIsClickable(int message_id) override { return true; }
+
+ private:
+  bool was_clicked_ = false;
+};
+
+}  // namespace

[Kyle Horimoto, 2017/07/12 22:45:59]

nit: Newline after this.

[lesliewatkins, 2017/07/13 22:12:49]

Done.

+namespace test {

[Kyle Horimoto, 2017/07/12 22:45:59]

Is there a reason why you're using this extra namespace? I haven't seen it
before in the Chromium code base. Did you copy this from another example?

If you aren't following a convention of other classes in this namespace, please
remove the extra namespace.

[lesliewatkins, 2017/07/13 22:12:48]

I moved the test namespace up to the top. The anonymous namespace is following
conventions as far as I can tell.

[Kyle Horimoto, 2017/07/13 23:15:36]

Sorry I wasn't clear - I'm referring to the test namespace (this test is
ash::test::TrayInfoLabelTest), not the anonymous namespace :)

Yesterday, Codesearch was down, so I couldn't search very well, but now that
it's up again, it looks like there are only a few unit tests using the test::
namespace with hundreds not using it in //ash/system. Let's go ahead and get rid
of that namespace since it seems non-standard.

[lesliewatkins, 2017/07/14 06:41:52]

Done.

+
+class TrayInfoLabelTest : public AshTestBase {

[Kyle Horimoto, 2017/07/12 22:45:59]

Add instance variables (std::unique_ptr's) for both the label and the delegate.

Then, add a CreateLabel(int message_id, bool use_delegate) function. In that
function, initialize your delegate if |use_delegate| is true, then initialize
your label.

Then, change your Confirm{Non}ClickableLabel() to:

void VerifyClickability(bool expected_clickable) {
  EXPECT_EQ(expected_clickable, label_->IsClickable());
}

Then, add a function to click:

void ClickOnLabel() {
  label_->PerformAction(TestClickEvent());
}

Then, add functions which verify clicks:

void VerifyClicks(const std::vector<int> expected_clicked_message_ids) {
  if (!delegate_) {
    EXPECT_TRUE(expected_clicked_message_ids.empty());
    return;
  }

  EXPECT_EQ(expected_clicked_message_ids, delegate_->clicked_message_ids());
}

void VerifyNoClicks() {
  VerifyClicks(std::vector<int>());
}

Your tests then turn into something like:

CreateLabel(1 /* message_id */, false /* use_delegate */);
VerifyClickability(false);
ClickOnLabel();
VerifyNoClicks();

or

CreateLabel(1 /* message_id */, true /* use_delegate */);
delegate_->AddClickableMessageId(1);
VerifyClickability(true);
ClickOnLabel();
VerifyClicks(std::vector<int> { 1 });

[lesliewatkins, 2017/07/13 22:12:48]

Done.

+ public:
+  int LabelMessageId(TrayInfoLabel* label) { return label->message_id_; }
+
+  void ConfirmClickableLabel(TrayInfoLabel* label) {
+    EXPECT_TRUE(label->IsClickable());
+  }
+
+  void ConfirmNonClickableLabel(TrayInfoLabel* label) {
+    EXPECT_FALSE(label->IsClickable());
+  }
+};
+
+TEST_F(TrayInfoLabelTest, NoDelegate) {
+  TrayInfoLabel* label = new TrayInfoLabel(nullptr, 0);
+
+  ConfirmNonClickableLabel(label);
+}
+
+TEST_F(TrayInfoLabelTest, NonClickableDelegate) {

[Kyle Horimoto, 2017/07/12 22:45:59]

For the tests with the delegate, call Update() with different message IDs and
assert that the clickability changes and that clicks are handled properly. Test
multiple clicks, clicks on different message IDs, etc.

You also need to add tests which verify that the style was updated properly and
that the a11y stuff is correct after Update() causes them to change.

[lesliewatkins, 2017/07/13 22:12:48]

That's what the Update test does.
Can you give me an example of another system tray element that tests style and
a11y? It seems like pretty non-standard practice, and it's going to be difficult
and time consuming to implement, to the point that it may not be worth it. I'm
happy to be shown otherwise though.

[Kyle Horimoto, 2017/07/13 23:15:36]

You just need to show that the correct values are returned by the label. To do
so, modify VerifyClickability():

void VerifyClickability(bool expected_clickable) {
  EXPECT_EQ(expected_clickable, label_->IsClickable());

  ui::AXNodeData node_data;
  label_->GetAccessibleNodeData(&node_data);

  if (expected_clickable) {
    EXPECT_EQ(ui::AX_ROLE_BUTTON, node_data.role);
    EXPECT_EQ(<clickableFontList>, label_->font_list());
    EXPECT_EQ(<clickableEnabledColor> label_->enabled_color());
    EXPECT_EQ(<clickableSelectionTextColor> label_->selection_text_color());
  } else {
    EXPECT_EQ(ui::AX_ROLE_LABEL_TEXT, node_data.role);
    EXPECT_EQ(<clickableFontList>, label_->font_list());
    EXPECT_EQ(<clickableEnabledColor> label_->enabled_color());
    EXPECT_EQ(<clickableSelectionTextColor> label_->selection_text_color());
  }
}

(You'll have to figure out the proper values for the things I put in <>'s. You
can find the values in your changes to tray_popup_item_style.cc.)

As an alternative to the if/else above, you could calculate all the variables
ahead of time and only call the EXPECT_EQ() once (instead of once in the "if"
and once in the "else"). Example:

AXRole expected_role = expected_clickable ?
    ui::AX_ROLE_BUTTON : ui::AX_ROLE_LABEL_TEXT;
/* Create other expected variables... */

EXPECT_EQ(expected_role, node_data.role);
/* Verify other expected values against label_... */

[lesliewatkins, 2017/07/14 06:41:51]

I added the AX node stuff, but still not convinced that checking the font and
style is the purpose of this test. At any rate, FontList doesn't seem to even
have a comparison operator.

[Kyle Horimoto, 2017/07/14 17:54:21]

Okay, that's fine! The a11y stuff was the bigger concern of mine.

[lesliewatkins, 2017/07/14 20:43:16]

Acknowledged.

+  TrayInfoLabel* label = new TrayInfoLabel(new TestNonClickableDelegate(), 0);
+
+  ConfirmNonClickableLabel(label);
+}
+
+TEST_F(TrayInfoLabelTest, ClickableDelegate) {
+  TrayInfoLabel* label = new TrayInfoLabel(new TestClickableDelegate(), 0);
+
+  ConfirmClickableLabel(label);
+}
+
+TEST_F(TrayInfoLabelTest, PerformAction) {
+  TestClickableDelegate* delegate = new TestClickableDelegate();
+  TrayInfoLabel* label = new TrayInfoLabel(delegate, 0);
+
+  ASSERT_FALSE(delegate->LabelWasClicked());
+
+  TestEvent event;
+  label->PerformAction(event);
+
+  EXPECT_TRUE(delegate->LabelWasClicked());
+}
+
+TEST_F(TrayInfoLabelTest, Update) {
+  TrayInfoLabel* label = new TrayInfoLabel(nullptr, 0);
+  ASSERT_EQ(LabelMessageId(label), 0);
+
+  label->Update(1);
+  EXPECT_EQ(LabelMessageId(label), 1);
+}
+
+}  // namespace test
+}  // namespace ash