[Spellcheck] Fixed subtly broken test.

chrome/renderer/spellchecker/spellcheck_worditerator_unittest.cc

 // Copyright (c) 2012 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 <stddef.h>
 
 #include <string>
 #include <vector>
 
 #include "base/format_macros.h"
 #include "base/i18n/break_iterator.h"
 #include "base/macros.h"
 #include "base/strings/string_split.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/renderer/spellchecker/spellcheck_worditerator.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::i18n::BreakIterator;
+using WordIteratorStatus = SpellcheckWordIterator::WordIteratorStatus;
 
 namespace {
 
 struct TestCase {
     const char* language;
     bool allow_contraction;
     const wchar_t* expected_words;
 };
 
 base::string16 GetRulesForLanguage(const std::string& language) {
   SpellcheckCharAttribute attribute;
   attribute.SetDefaultLanguage(language);
   return attribute.GetRuleSet(true);
 }
 
+WordIteratorStatus GetNextNonSkippableWord(SpellcheckWordIterator* iterator,
+                                           base::string16* word_string,
+                                           int* word_start,
+                                           int* word_length) {
+  WordIteratorStatus status = SpellcheckWordIterator::IS_SKIPPABLE;
+  while (status == SpellcheckWordIterator::IS_SKIPPABLE)
+    status = iterator->GetNextWord(word_string, word_start, word_length);

[Nico, 2016/01/06 01:38:26]

with a do-while loop you don't need to initialize status to a default

[groby-ooo-7-16, 2016/01/06 01:48:37]

True, but it's one more line :) See below:

Status status;
do {
  status = GetNext();
} while (status == IS_SKIPPABLE);

vs.

Status status = SKIPPABLE
while (status == SKPPABLE)
  status = GetNext();

I could've of course opted for the *truly* horrible solution ;)

Status status;
for (;(status=GetNext()) == IS_SKIPPABLE;) {}

[Nico, 2016/01/06 02:36:16]

<s>More horrible</s>You don't need the parens:

  Status status;
  do status = GetNext();
  while (status == IS_SKIPPABLE);

+  return status;
+}
+
 }  // namespace
 
 // Tests whether or not our SpellcheckWordIterator can extract words used by the
 // specified language from a multi-language text.
 TEST(SpellcheckWordIteratorTest, SplitWord) {
   // An input text. This text includes words of several languages. (Some words
   // are not separated with whitespace characters.) Our SpellcheckWordIterator
   // should extract the words used by the specified language from this text and
   // normalize them so our spell-checker can check their spellings. If
   // characters are found that are not from the specified language the test
@@ skipping 92 matching lines @@
     SpellcheckWordIterator iterator;
     EXPECT_TRUE(iterator.Initialize(&attributes,
                                     kTestCases[i].allow_contraction));
     EXPECT_TRUE(iterator.SetText(input.c_str(), input.length()));
 
     std::vector<base::string16> expected_words = base::SplitString(
         base::WideToUTF16(kTestCases[i].expected_words),
         base::string16(1, ' '), base::TRIM_WHITESPACE, base::SPLIT_WANT_ALL);
 
     base::string16 actual_word;
-    int actual_start, actual_end;
+    int actual_start, actual_len;
     size_t index = 0;
     for (SpellcheckWordIterator::WordIteratorStatus status =
-             iterator.GetNextWord(&actual_word, &actual_start, &actual_end);
+             iterator.GetNextWord(&actual_word, &actual_start, &actual_len);
          status != SpellcheckWordIterator::IS_END_OF_TEXT;
          status =
-             iterator.GetNextWord(&actual_word, &actual_start, &actual_end)) {
+             iterator.GetNextWord(&actual_word, &actual_start, &actual_len)) {
       if (status == SpellcheckWordIterator::WordIteratorStatus::IS_SKIPPABLE)
         continue;
 
       EXPECT_TRUE(index < expected_words.size());
       if (index < expected_words.size())
         EXPECT_EQ(expected_words[index], actual_word);
       ++index;
     }
   }
 }
 
 // Tests whether our SpellcheckWordIterator extracts an empty word without
 // getting stuck in an infinite loop when inputting a Khmer text. (This is a
 // regression test for Issue 46278.)
 TEST(SpellcheckWordIteratorTest, RuleSetConsistency) {
   SpellcheckCharAttribute attributes;
   attributes.SetDefaultLanguage("en-US");
 
   const wchar_t kTestText[] = L"\x1791\x17c1\x002e";
   base::string16 input(base::WideToUTF16(kTestText));
 
   SpellcheckWordIterator iterator;
   EXPECT_TRUE(iterator.Initialize(&attributes, true));
   EXPECT_TRUE(iterator.SetText(input.c_str(), input.length()));
 
   // When SpellcheckWordIterator uses an inconsistent ICU ruleset, the following
   // iterator.GetNextWord() calls get stuck in an infinite loop. Therefore, this
   // test succeeds if this call returns without timeouts.
   base::string16 actual_word;
-  int actual_start, actual_end;
-  SpellcheckWordIterator::WordIteratorStatus status;
-  for (status = iterator.GetNextWord(&actual_word, &actual_start, &actual_end);
-       status == SpellcheckWordIterator::IS_SKIPPABLE;
-       status =
-           iterator.GetNextWord(&actual_word, &actual_start, &actual_end)) {
-    continue;
-  }
+  int actual_start, actual_len;
+  WordIteratorStatus status = GetNextNonSkippableWord(
+      &iterator, &actual_word, &actual_start, &actual_len);
 
   EXPECT_EQ(SpellcheckWordIterator::WordIteratorStatus::IS_END_OF_TEXT, status);
   EXPECT_EQ(0, actual_start);
-  EXPECT_EQ(0, actual_end);
+  EXPECT_EQ(0, actual_len);
 }
 
 // Vertify our SpellcheckWordIterator can treat ASCII numbers as word characters
 // on LTR languages. On the other hand, it should not treat ASCII numbers as
 // word characters on RTL languages because they change the text direction from
 // RTL to LTR.
 TEST(SpellcheckWordIteratorTest, TreatNumbersAsWordCharacters) {
   // A set of a language, a dummy word, and a text direction used in this test.
   // For each language, this test splits a dummy word, which consists of ASCII
   // numbers and an alphabet of the language, into words. When ASCII numbers are
@@ skipping 37 matching lines @@
 
     SpellcheckCharAttribute attributes;
     attributes.SetDefaultLanguage(kTestCases[i].language);
 
     base::string16 input_word(base::WideToUTF16(kTestCases[i].text));
     SpellcheckWordIterator iterator;
     EXPECT_TRUE(iterator.Initialize(&attributes, true));
     EXPECT_TRUE(iterator.SetText(input_word.c_str(), input_word.length()));
 
     base::string16 actual_word;
-    int actual_start, actual_end;
-    SpellcheckWordIterator::WordIteratorStatus status;
-    for (status =
-             iterator.GetNextWord(&actual_word, &actual_start, &actual_end);
-         status == SpellcheckWordIterator::IS_SKIPPABLE;
-         status =
-             iterator.GetNextWord(&actual_word, &actual_start, &actual_end)) {
-      continue;
-    }
+    int actual_start, actual_len;
+    WordIteratorStatus status = GetNextNonSkippableWord(
+        &iterator, &actual_word, &actual_start, &actual_len);
 
     EXPECT_EQ(SpellcheckWordIterator::WordIteratorStatus::IS_WORD, status);
     if (kTestCases[i].left_to_right)
       EXPECT_EQ(input_word, actual_word);
     else
       EXPECT_NE(input_word, actual_word);
   }
 }
 
-// Vertify SpellcheckWordIterator treats typographical apostrophe as a part of
+// Verify SpellcheckWordIterator treats typographical apostrophe as a part of
 // the word.
 TEST(SpellcheckWordIteratorTest, TypographicalApostropheIsPartOfWord) {
   static const struct {
     const char* language;
-    const wchar_t* word;
+    const wchar_t* input;
+    const wchar_t* expected;
   } kTestCases[] = {
-    // Typewriter apostrophe:
-    {
-      "en-AU", L"you're"
-    }, {
-      "en-CA", L"you're"
-    }, {
-      "en-GB", L"you're"
-    }, {
-      "en-US", L"you're"
-    },
-    // Typographical apostrophe:
-    {
-      "en-AU", L"you\x2019re"
-    }, {
-      "en-CA", L"you\x2019re"
-    }, {
-      "en-GB", L"you\x2019re"
-    }, {
-      "en-US", L"you\x2019re"
-    },
+      // Typewriter apostrophe:
+      {"en-AU", L"you're", L"you're"},
+      {"en-CA", L"you're", L"you're"},
+      {"en-GB", L"you're", L"you're"},
+      {"en-US", L"you're", L"you're"},
+      {"en-US", L"!!!!you're", L"you're"},
+      // Typographical apostrophe:
+      {"en-AU", L"you\x2019re", L"you\x2019re"},
+      {"en-CA", L"you\x2019re", L"you\x2019re"},
+      {"en-GB", L"you\x2019re", L"you\x2019re"},
+      {"en-US", L"you\x2019re", L"you\x2019re"},
+      {"en-US", L"....you\x2019re", L"you\x2019re"},
   };
 
   for (size_t i = 0; i < arraysize(kTestCases); ++i) {
     SpellcheckCharAttribute attributes;
     attributes.SetDefaultLanguage(kTestCases[i].language);
 
-    base::string16 input_word(base::WideToUTF16(kTestCases[i].word));
+    base::string16 input_word(base::WideToUTF16(kTestCases[i].input));
+    base::string16 expected_word(base::WideToUTF16(kTestCases[i].expected));
     SpellcheckWordIterator iterator;
     EXPECT_TRUE(iterator.Initialize(&attributes, true));
     EXPECT_TRUE(iterator.SetText(input_word.c_str(), input_word.length()));
 
     base::string16 actual_word;
-    int actual_start, actual_end;
-    SpellcheckWordIterator::WordIteratorStatus status;
-    for (status =
-             iterator.GetNextWord(&actual_word, &actual_start, &actual_end);
-         status == SpellcheckWordIterator::IS_SKIPPABLE;
-         iterator.GetNextWord(&actual_word, &actual_start, &actual_end)) {
-      continue;
-    }
+    int actual_start, actual_len;
+    WordIteratorStatus status = GetNextNonSkippableWord(
+        &iterator, &actual_word, &actual_start, &actual_len);
 
     EXPECT_EQ(SpellcheckWordIterator::WordIteratorStatus::IS_WORD, status);
-    EXPECT_EQ(input_word, actual_word);
-    EXPECT_EQ(0, actual_start);
-    EXPECT_EQ(input_word.length(),
-              static_cast<base::string16::size_type>(actual_end));
+    EXPECT_EQ(expected_word, actual_word);
+    EXPECT_LE(0, actual_start);
+    EXPECT_EQ(expected_word.length(),
+              static_cast<base::string16::size_type>(actual_len));
   }
 }
 
 TEST(SpellcheckWordIteratorTest, Initialization) {
   // Test initialization works when a default language is set.
   {
     SpellcheckCharAttribute attributes;
     attributes.SetDefaultLanguage("en-US");
 
     SpellcheckWordIterator iterator;
@@ skipping 158 matching lines @@
   EXPECT_EQ(base::UTF8ToUTF16("."), iter.GetString());
   EXPECT_EQ(iter.GetWordBreakStatus(), BreakIterator::IS_SKIPPABLE_WORD);
   EXPECT_TRUE(iter.Advance());
   EXPECT_EQ(base::UTF8ToUTF16(" "), iter.GetString());
   EXPECT_EQ(iter.GetWordBreakStatus(), BreakIterator::IS_SKIPPABLE_WORD);
   EXPECT_TRUE(iter.Advance());
   EXPECT_EQ(base::UTF8ToUTF16(","), iter.GetString());
   EXPECT_EQ(iter.GetWordBreakStatus(), BreakIterator::IS_SKIPPABLE_WORD);
   EXPECT_FALSE(iter.Advance());
 }