[sql] sql::ScopedErrorIgnorer rename to sql::test::ScopedErrorExpecter

sql/test/scoped_error_expecter.cc

+// Copyright 2013 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 "sql/test/scoped_error_expecter.h"
+
+#include "base/bind.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace sql {
+namespace test {
+
+// static
+int ScopedErrorExpecter::SQLiteLibVersionNumber() {
+  return sqlite3_libversion_number();
+}
+
+ScopedErrorExpecter::ScopedErrorExpecter()
+    : checked_(false) {
+  callback_ =
+      base::Bind(&ScopedErrorExpecter::ErrorSeen, base::Unretained(this));
+  Connection::SetErrorExpecter(&callback_);
+}
+
+ScopedErrorExpecter::~ScopedErrorExpecter() {
+  EXPECT_TRUE(checked_) << " Test must call SawExpectedErrors()";
+  Connection::ResetErrorExpecter();
+}
+
+void ScopedErrorExpecter::ExpectError(int err) {
+  EXPECT_EQ(0u, errors_expected_.count(err))
+      << " Error " << err << " is already expected";
+  errors_expected_.insert(err);
+}
+
+bool ScopedErrorExpecter::SawExpectedErrors() {
+  checked_ = true;
+  return errors_expected_ == errors_seen_;
+}
+
+bool ScopedErrorExpecter::ErrorSeen(int err) {
+  // Look for extended code.
+  if (errors_expected_.count(err) > 0) {
+    // Record that the error was seen.
+    errors_seen_.insert(err);
+    return true;
+  }
+
+  // Trim extended codes and check again.
+  int base_err = err & 0xff;
+  if (errors_expected_.count(base_err) > 0) {
+    // Record that the error was seen.
+    errors_seen_.insert(base_err);
+    return true;
+  }
+
+  // Unexpected error.
+  ADD_FAILURE() << " Unexpected SQLite error " << err;
+  return false;

[Mark P, 2016/05/23 19:53:27]

You changed this from false to true.  I assume it was intentional and there is
no other code that needs to be updated... ? (as no code should actually reach
here instead of failing above)

[Scott Hess - ex-Googler, 2016/06/03 00:02:12]

Previously all cases returned true, which meant that unexpected errors were
collected by ADD_FAILURE() and reported at the final "the right errors were
seen" expectation.  But that just indicated that new errors were happening, it
didn't tell where they were.  Returning false means the calling code will call
the DLOG(FATAL) case, which will provide a useful backtrace.  I think that's
what I originally intended, based on how Connection::ShouldIgnoreError() was
written.

ADD_FAILURE() would be preferred, DLOG(FATAL) is generally not preferred for
tests, but AFAICT ADD_FAILURE() can't provide backtrace info.  I'm trying to
think of a way to square the circle, but can't.  Since this case can only happen
if someone is not running tests on their local machine, I'm inclined to just go
with the useful crash, so that what to fix it obvious.

[Mark P, 2016/06/15 04:39:44]

Acknowledged.

+}
+
+}  // namespace test
+}  // namespace sql