Add iterator for DocumentMarker::MarkerTypes

third_party/WebKit/Source/core/editing/markers/DocumentMarkerTest.cpp

Index: third_party/WebKit/Source/core/editing/markers/DocumentMarkerTest.cpp
diff --git a/third_party/WebKit/Source/core/editing/markers/DocumentMarkerTest.cpp b/third_party/WebKit/Source/core/editing/markers/DocumentMarkerTest.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..c9d2110ea3cd4ba0139be60686f76b7948d4601d
--- /dev/null
+++ b/third_party/WebKit/Source/core/editing/markers/DocumentMarkerTest.cpp
@@ -0,0 +1,29 @@
+// 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 "core/editing/markers/DocumentMarker.h"
+
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+
+class DocumentMarkerTest : public ::testing::Test {};
+
+TEST_F(DocumentMarkerTest, MarkerTypesIterator) {
+  for (unsigned i = 0; i < (1 << DocumentMarker::MarkerTypeIndexesCount); ++i) {
+    DocumentMarker::MarkerTypes markerTypes(i);
+    Vector<DocumentMarker::MarkerType> typesFromIterator;
+
+    for (DocumentMarker::MarkerType type : markerTypes) {

[Xiaocheng, 2017/03/22 01:02:58]

nit: unnecessary braces.

+      typesFromIterator.push_back(type);
+    }
+
+    for (int i = 0; i < DocumentMarker::MarkerTypeIndexesCount; ++i) {

[Xiaocheng, 2017/03/22 01:47:12]

s/int/unsigned/, and use another variable name.

+      DocumentMarker::MarkerType type =
+          static_cast<DocumentMarker::MarkerType>(1 << i);
+      ASSERT_EQ(markerTypes.contains(type), typesFromIterator.contains(type));

[Xiaocheng, 2017/03/22 01:02:58]

I'm confused...

yosin@: Is there any preference between EXPECT_FOO and ASSERT_FOO in unit tests?

[rlanday, 2017/03/22 01:38:25]

This checks that for each possible combination of MarkerTypes, the set of types
the iterator goes through is correct (by using MarkerTypes.contains() which we
assume already works).

I used ASSERT_EQ since if this test fails and we didn't immediately stop, there
would be a ton of not-very-helpful output since this is inside two nested loops.

[Xiaocheng, 2017/03/22 01:47:12]

Makes sense to use ASSERT_EQ here.

To make the test useful, however, the fail message should also contain the value
of |i| and |i| (just noticed that you used the same loop variable name...).

+    }
+  }
+}
+}