Make conformance tests more strict about the failure list.

The failure lists were falling out of date because the
tests would pass even if people forgot to remove failures
from the list.

conformance/conformance_test.cc

@@ -30,6 +30,7 @@
 
 #include <stdarg.h>
 #include <string>
+#include <fstream>
 
 #include "conformance.pb.h"
 #include "conformance_test.h"
@@ -575,24 +576,41 @@ void ConformanceTestSuite::TestPrematureEOFForType(FieldDescriptor::Type type) {
   }
 }
 
-void ConformanceTestSuite::SetFailureList(const vector<string>& failure_list) {
+void ConformanceTestSuite::SetFailureList(const string& filename,
+                                          const vector<string>& failure_list) {
+  failure_list_filename_ = filename;
   expected_to_fail_.clear();
   std::copy(failure_list.begin(), failure_list.end(),
             std::inserter(expected_to_fail_, expected_to_fail_.end()));
 }
 
 bool ConformanceTestSuite::CheckSetEmpty(const set<string>& set_to_check,
-                                         const char* msg) {
+                                         const std::string& write_to_file,
+                                         const std::string& msg) {
   if (set_to_check.empty()) {
     return true;
   } else {
     StringAppendF(&output_, "\n");
-    StringAppendF(&output_, "%s:\n", msg);
+    StringAppendF(&output_, "%s\n\n", msg.c_str());
     for (set<string>::const_iterator iter = set_to_check.begin();
          iter != set_to_check.end(); ++iter) {
       StringAppendF(&output_, "  %s\n", iter->c_str());
     }
     StringAppendF(&output_, "\n");
+
+    if (!write_to_file.empty()) {
+      std::ofstream os(write_to_file);
+      if (os) {
+        for (set<string>::const_iterator iter = set_to_check.begin();
+             iter != set_to_check.end(); ++iter) {
+          os << *iter << "\n";
+        }
+      } else {
+        StringAppendF(&output_, "Failed to open file: %s\n",
+                      write_to_file.c_str());
+      }
+    }
+
     return false;
   }
 }
@@ -1965,27 +1983,34 @@ bool ConformanceTestSuite::RunSuite(ConformanceTestRunner* runner,
       )");
 
   bool ok = true;
-  if (!CheckSetEmpty(expected_to_fail_,
+  if (!CheckSetEmpty(expected_to_fail_, "nonexistent_tests.txt",
                      "These tests were listed in the failure list, but they "
-                     "don't exist.  Remove them from the failure list")) {
+                     "don't exist.  Remove them from the failure list by "
+                     "running:\n"
+                     "  ./update_failure_list.py " + failure_list_filename_ +
+                     " --remove nonexistent_tests.txt")) {
     ok = false;
   }
-  if (!CheckSetEmpty(unexpected_failing_tests_,
+  if (!CheckSetEmpty(unexpected_failing_tests_, "failing_tests.txt",
                      "These tests failed.  If they can't be fixed right now, "
                      "you can add them to the failure list so the overall "
-                     "suite can succeed")) {
+                     "suite can succeed.  Add them to the failure list by "
+                     "running:\n"
+                     "  ./update_failure_list.py " + failure_list_filename_ +
+                     " --add failing_tests.txt")) {
+    ok = false;
+  }
+  if (!CheckSetEmpty(unexpected_succeeding_tests_, "succeeding_tests.txt",
+                     "These tests succeeded, even though they were listed in "
+                     "the failure list.  Remove them from the failure list "
+                     "by running:\n"
+                     "  ./update_failure_list.py " + failure_list_filename_ +
+                     " --remove succeeding_tests.txt")) {
     ok = false;
   }
-
-  // Sometimes the testee may be fixed before we update the failure list (e.g.,
-  // the testee is from a different component). We warn about this case but
-  // don't consider it an overall test failure.
-  CheckSetEmpty(unexpected_succeeding_tests_,
-                "These tests succeeded, even though they were listed in "
-                "the failure list.  Remove them from the failure list");
 
   if (verbose_) {
-    CheckSetEmpty(skipped_,
+    CheckSetEmpty(skipped_, "",
                   "These tests were skipped (probably because support for some "
                   "features is not implemented)");
   }

conformance/conformance_test.h

@@ -98,7 +98,11 @@ class ConformanceTestSuite {
   // Sets the list of tests that are expected to fail when RunSuite() is called.
   // RunSuite() will fail unless the set of failing tests is exactly the same
   // as this list.
-  void SetFailureList(const std::vector<std::string>& failure_list);
+  //
+  // The filename here is *only* used to create/format useful error messages for
+  // how to update the failure list.  We do NOT read this file at all.
+  void SetFailureList(const std::string& filename,
+                      const std::vector<std::string>& failure_list);
 
   // Run all the conformance tests against the given test runner.
   // Test output will be stored in "output".
@@ -143,12 +147,14 @@ class ConformanceTestSuite {
   void ExpectHardParseFailureForProto(const std::string& proto,
                                       const std::string& test_name);
   void TestPrematureEOFForType(google::protobuf::FieldDescriptor::Type type);
-  bool CheckSetEmpty(const set<string>& set_to_check, const char* msg);
+  bool CheckSetEmpty(const set<string>& set_to_check,
+                     const std::string& write_to_file, const std::string& msg);
   ConformanceTestRunner* runner_;
   int successes_;
   int expected_failures_;
   bool verbose_;
   std::string output_;
+  std::string failure_list_filename_;
 
   // The set of test names that are expected to fail in this run, but haven't
   // failed yet.

conformance/conformance_test_runner.cc

@@ -280,11 +280,13 @@ int main(int argc, char *argv[]) {
   char *program;
   google::protobuf::ConformanceTestSuite suite;
 
+  string failure_list_filename;
   vector<string> failure_list;
 
   for (int arg = 1; arg < argc; ++arg) {
     if (strcmp(argv[arg], "--failure_list") == 0) {
       if (++arg == argc) UsageError();
+      failure_list_filename = argv[arg];
       ParseFailureList(argv[arg], &failure_list);
     } else if (strcmp(argv[arg], "--verbose") == 0) {
       suite.SetVerbose(true);
@@ -300,7 +302,7 @@ int main(int argc, char *argv[]) {
     }
   }
 
-  suite.SetFailureList(failure_list);
+  suite.SetFailureList(failure_list_filename, failure_list);
   ForkPipeRunner runner(program);
 
   std::string output;

conformance/failure_list_cpp.txt

@@ -12,21 +12,11 @@ FieldMaskPathsDontRoundTrip.JsonOutput
 FieldMaskTooManyUnderscore.JsonOutput
 JsonInput.AnyUnorderedTypeTag.JsonOutput
 JsonInput.AnyUnorderedTypeTag.ProtobufOutput
-JsonInput.AnyWithValueForInteger.JsonOutput
-JsonInput.AnyWithValueForInteger.ProtobufOutput
-JsonInput.AnyWithValueForJsonObject.JsonOutput
-JsonInput.AnyWithValueForJsonObject.ProtobufOutput
 JsonInput.BoolFieldDoubleQuotedFalse
 JsonInput.BoolFieldDoubleQuotedTrue
-JsonInput.BoolFieldIntegerOne
-JsonInput.BoolFieldIntegerZero
-JsonInput.BytesFieldInvalidBase64Characters
 JsonInput.BytesFieldNoPadding
 JsonInput.DoubleFieldTooSmall
 JsonInput.DurationHasZeroFractionalDigit.Validator
-JsonInput.DurationJsonInputTooLarge
-JsonInput.DurationJsonInputTooSmall
-JsonInput.DurationMissingS
 JsonInput.EnumFieldUnknownValue.Validator
 JsonInput.FieldMaskInvalidCharacter
 JsonInput.FieldNameDuplicate
@@ -36,58 +26,12 @@ JsonInput.FieldNameInLowerCamelCase.Validator
 JsonInput.FieldNameInSnakeCase.JsonOutput
 JsonInput.FieldNameInSnakeCase.ProtobufOutput
 JsonInput.FieldNameNotQuoted
-JsonInput.FloatFieldTooLarge
-JsonInput.FloatFieldTooSmall
-JsonInput.Int32FieldLeadingSpace
-JsonInput.Int32FieldLeadingZero
-JsonInput.Int32FieldMinFloatValue.JsonOutput
-JsonInput.Int32FieldMinFloatValue.ProtobufOutput
-JsonInput.Int32FieldMinValue.JsonOutput
-JsonInput.Int32FieldMinValue.ProtobufOutput
-JsonInput.Int32FieldNegativeWithLeadingZero
-JsonInput.Int32FieldNotInteger
-JsonInput.Int32FieldNotNumber
-JsonInput.Int32FieldTooLarge
-JsonInput.Int32FieldTooSmall
-JsonInput.Int32FieldTrailingSpace
-JsonInput.Int64FieldNotInteger
-JsonInput.Int64FieldNotNumber
-JsonInput.Int64FieldTooLarge
-JsonInput.Int64FieldTooSmall
 JsonInput.MapFieldValueIsNull
-JsonInput.OneofFieldDuplicate
 JsonInput.RepeatedFieldMessageElementIsNull
 JsonInput.RepeatedFieldPrimitiveElementIsNull
 JsonInput.RepeatedFieldTrailingComma
-JsonInput.RepeatedFieldWrongElementTypeExpectingIntegersGotBool
-JsonInput.RepeatedFieldWrongElementTypeExpectingIntegersGotMessage
-JsonInput.RepeatedFieldWrongElementTypeExpectingIntegersGotString
-JsonInput.RepeatedFieldWrongElementTypeExpectingMessagesGotBool
-JsonInput.RepeatedFieldWrongElementTypeExpectingMessagesGotInt
-JsonInput.RepeatedFieldWrongElementTypeExpectingMessagesGotString
-JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotBool
-JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotInt
-JsonInput.RepeatedFieldWrongElementTypeExpectingStringsGotMessage
-JsonInput.StringFieldNotAString
-JsonInput.StringFieldSurrogateInWrongOrder
-JsonInput.StringFieldSurrogatePair.JsonOutput
-JsonInput.StringFieldSurrogatePair.ProtobufOutput
-JsonInput.StringFieldUnpairedHighSurrogate
-JsonInput.StringFieldUnpairedLowSurrogate
 JsonInput.StringFieldUppercaseEscapeLetter
-JsonInput.TimestampJsonInputLowercaseT
-JsonInput.TimestampJsonInputLowercaseZ
-JsonInput.TimestampJsonInputMissingT
-JsonInput.TimestampJsonInputMissingZ
-JsonInput.TimestampJsonInputTooLarge
-JsonInput.TimestampJsonInputTooSmall
 JsonInput.TrailingCommaInAnObject
-JsonInput.Uint32FieldNotInteger
-JsonInput.Uint32FieldNotNumber
-JsonInput.Uint32FieldTooLarge
-JsonInput.Uint64FieldNotInteger
-JsonInput.Uint64FieldNotNumber
-JsonInput.Uint64FieldTooLarge
 JsonInput.WrapperTypesWithNullValue.JsonOutput
 JsonInput.WrapperTypesWithNullValue.ProtobufOutput
 ProtobufInput.PrematureEofBeforeKnownRepeatedValue.MESSAGE
@@ -102,5 +46,3 @@ ProtobufInput.PrematureEofInPackedField.SINT64
 ProtobufInput.PrematureEofInPackedField.UINT32
 ProtobufInput.PrematureEofInPackedField.UINT64
 ProtobufInput.PrematureEofInsideKnownRepeatedValue.MESSAGE
-TimestampProtoInputTooLarge.JsonOutput
-TimestampProtoInputTooSmall.JsonOutput

conformance/failure_list_java.txt

@@ -8,8 +8,6 @@ FieldMaskNumbersDontRoundTrip.JsonOutput
 FieldMaskPathsDontRoundTrip.JsonOutput
 FieldMaskTooManyUnderscore.JsonOutput
 JsonInput.AnyWithFieldMask.ProtobufOutput
-JsonInput.AnyWithValueForInteger.JsonOutput
-JsonInput.AnyWithValueForJsonObject.JsonOutput
 JsonInput.BoolFieldAllCapitalFalse
 JsonInput.BoolFieldAllCapitalTrue
 JsonInput.BoolFieldCamelCaseFalse
@@ -30,8 +28,6 @@ JsonInput.FloatFieldInfinityNotQuoted
 JsonInput.FloatFieldNanNotQuoted
 JsonInput.FloatFieldNegativeInfinityNotQuoted
 JsonInput.Int32FieldLeadingZero
-JsonInput.Int32FieldMinFloatValue.JsonOutput
-JsonInput.Int32FieldMinValue.JsonOutput
 JsonInput.Int32FieldNegativeWithLeadingZero
 JsonInput.Int32FieldPlusSign
 JsonInput.Int32MapFieldKeyNotQuoted