Conformance tests can now be excluded based on their names.

This allows us to enable conformance tests even when we know
that some tests are failing and need to be fixed.

Change-Id: I372f43663008747db6f2b2cf06e6ffa4c6d85b2d

conformance/Makefile.am

@@ -57,7 +57,7 @@ conformance-java: javac_middleman
 
 # Targets for actually running tests.
 test_cpp: protoc_middleman conformance-test-runner conformance-cpp
-	./conformance-test-runner ./conformance-cpp
+	./conformance-test-runner --failure_list failure_list_cpp.txt ./conformance-cpp
 
 test_java: protoc_middleman conformance-test-runner conformance-java
 	./conformance-test-runner ./conformance-java

conformance/conformance.proto

@@ -57,11 +57,13 @@ option java_package = "com.google.protobuf.conformance";
 //   2. parse the protobuf or JSON payload in "payload" (which may fail)
 //   3. if the parse succeeded, serialize the message in the requested format.
 message ConformanceRequest {
+  string test_name = 1;
+
   // The payload (whether protobuf of JSON) is always for a TestAllTypes proto
   // (see below).
   oneof payload {
-    bytes protobuf_payload = 1;
-    string json_payload = 2;
+    bytes protobuf_payload = 2;
+    string json_payload = 3;
   }
 
   enum RequestedOutput {
@@ -71,7 +73,7 @@ message ConformanceRequest {
   }
 
   // Which format should the testee serialize its message to?
-  RequestedOutput requested_output = 3;
+  RequestedOutput requested_output = 4;
 }
 
 // Represents a single test case's output.

conformance/conformance_test.cc

@@ -126,12 +126,11 @@ string submsg(uint32_t fn, const string& buf) {
 
 #define UNKNOWN_FIELD 666
 
-uint32_t GetFieldNumberForType(WireFormatLite::FieldType type, bool repeated) {
+uint32_t GetFieldNumberForType(FieldDescriptor::Type type, bool repeated) {
   const Descriptor* d = TestAllTypes().GetDescriptor();
   for (int i = 0; i < d->field_count(); i++) {
     const FieldDescriptor* f = d->field(i);
-    if (static_cast<WireFormatLite::FieldType>(f->type()) == type &&
-        f->is_repeated() == repeated) {
+    if (f->type() == type && f->is_repeated() == repeated) {
       return f->number();
     }
   }
@@ -139,16 +138,37 @@ uint32_t GetFieldNumberForType(WireFormatLite::FieldType type, bool repeated) {
   return 0;
 }
 
+string UpperCase(string str) {
+  for (int i = 0; i < str.size(); i++) {
+    str[i] = toupper(str[i]);
+  }
+  return str;
+}
+
 }  // anonymous namespace
 
 namespace google {
 namespace protobuf {
 
-void ConformanceTestSuite::ReportSuccess() {
+void ConformanceTestSuite::ReportSuccess(const string& test_name) {
+  if (expected_to_fail_.erase(test_name) != 0) {
+    StringAppendF(&output_,
+                  "ERROR: test %s is in the failure list, but test succeeded.  "
+                  "Remove it from the failure list.\n",
+                  test_name.c_str());
+    unexpected_succeeding_tests_.insert(test_name);
+  }
   successes_++;
 }
 
-void ConformanceTestSuite::ReportFailure(const char *fmt, ...) {
+void ConformanceTestSuite::ReportFailure(const string& test_name,
+                                         const char* fmt, ...) {
+  if (expected_to_fail_.erase(test_name) == 1) {
+    StringAppendF(&output_, "FAILED AS EXPECTED: ");
+  } else {
+    StringAppendF(&output_, "ERROR: ");
+    unexpected_failing_tests_.insert(test_name);
+  }
   va_list args;
   va_start(args, fmt);
   StringAppendV(&output_, fmt, args);
@@ -158,6 +178,10 @@ void ConformanceTestSuite::ReportFailure(const char *fmt, ...) {
 
 void ConformanceTestSuite::RunTest(const ConformanceRequest& request,
                                    ConformanceResponse* response) {
+  if (test_names_.insert(request.test_name()).second == false) {
+    GOOGLE_LOG(FATAL) << "Duplicated test name: " << request.test_name();
+  }
+
   string serialized_request;
   string serialized_response;
   request.SerializeToString(&serialized_request);
@@ -176,10 +200,12 @@ void ConformanceTestSuite::RunTest(const ConformanceRequest& request,
   }
 }
 
-void ConformanceTestSuite::DoExpectParseFailureForProto(const string& proto,
-                                                        int line) {
+// Expect that this precise protobuf will cause a parse error.
+void ConformanceTestSuite::ExpectParseFailureForProto(
+    const string& proto, const string& test_name) {
   ConformanceRequest request;
   ConformanceResponse response;
+  request.set_test_name(test_name);
   request.set_protobuf_payload(proto);
 
   // We don't expect output, but if the program erroneously accepts the protobuf
@@ -188,29 +214,27 @@ void ConformanceTestSuite::DoExpectParseFailureForProto(const string& proto,
 
   RunTest(request, &response);
   if (response.result_case() == ConformanceResponse::kParseError) {
-    ReportSuccess();
+    ReportSuccess(test_name);
   } else {
-    ReportFailure("Should have failed, but didn't. Line: %d, Request: %s, "
+    ReportFailure(test_name,
+                  "Should have failed to parse, but didn't. Request: %s, "
                   "response: %s\n",
-                  line,
                   request.ShortDebugString().c_str(),
                   response.ShortDebugString().c_str());
   }
 }
 
-// Expect that this precise protobuf will cause a parse error.
-#define ExpectParseFailureForProto(proto) DoExpectParseFailureForProto(proto, __LINE__)
-
 // Expect that this protobuf will cause a parse error, even if it is followed
 // by valid protobuf data.  We can try running this twice: once with this
 // data verbatim and once with this data followed by some valid data.
 //
 // TODO(haberman): implement the second of these.
-#define ExpectHardParseFailureForProto(proto) DoExpectParseFailureForProto(proto, __LINE__)
-
+void ConformanceTestSuite::ExpectHardParseFailureForProto(
+    const string& proto, const string& test_name) {
+  return ExpectParseFailureForProto(proto, test_name);
+}
 
-void ConformanceTestSuite::TestPrematureEOFForType(
-    WireFormatLite::FieldType type) {
+void ConformanceTestSuite::TestPrematureEOFForType(FieldDescriptor::Type type) {
   // Incomplete values for each wire type.
   static const string incompletes[6] = {
     string("\x80"),     // VARINT
@@ -223,45 +247,51 @@ void ConformanceTestSuite::TestPrematureEOFForType(
 
   uint32_t fieldnum = GetFieldNumberForType(type, false);
   uint32_t rep_fieldnum = GetFieldNumberForType(type, true);
-  WireFormatLite::WireType wire_type =
-      WireFormatLite::WireTypeForFieldType(type);
+  WireFormatLite::WireType wire_type = WireFormatLite::WireTypeForFieldType(
+      static_cast<WireFormatLite::FieldType>(type));
   const string& incomplete = incompletes[wire_type];
+  const string type_name =
+      UpperCase(string(".") + FieldDescriptor::TypeName(type));
 
-  // EOF before a known non-repeated value.
-  ExpectParseFailureForProto(tag(fieldnum, wire_type));
+  ExpectParseFailureForProto(
+      tag(fieldnum, wire_type),
+      "PrematureEofBeforeKnownNonRepeatedValue" + type_name);
 
-  // EOF before a known repeated value.
-  ExpectParseFailureForProto(tag(rep_fieldnum, wire_type));
+  ExpectParseFailureForProto(
+      tag(rep_fieldnum, wire_type),
+      "PrematureEofBeforeKnownRepeatedValue" + type_name);
 
-  // EOF before an unknown value.
-  ExpectParseFailureForProto(tag(UNKNOWN_FIELD, wire_type));
+  ExpectParseFailureForProto(
+      tag(UNKNOWN_FIELD, wire_type),
+      "PrematureEofBeforeUnknownValue" + type_name);
 
-  // EOF inside a known non-repeated value.
   ExpectParseFailureForProto(
-      cat( tag(fieldnum, wire_type), incomplete ));
+      cat( tag(fieldnum, wire_type), incomplete ),
+      "PrematureEofInsideKnownNonRepeatedValue" + type_name);
 
-  // EOF inside a known repeated value.
   ExpectParseFailureForProto(
-      cat( tag(rep_fieldnum, wire_type), incomplete ));
+      cat( tag(rep_fieldnum, wire_type), incomplete ),
+      "PrematureEofInsideKnownRepeatedValue" + type_name);
 
-  // EOF inside an unknown value.
   ExpectParseFailureForProto(
-      cat( tag(UNKNOWN_FIELD, wire_type), incomplete ));
+      cat( tag(UNKNOWN_FIELD, wire_type), incomplete ),
+      "PrematureEofInsideUnknownValue" + type_name);
 
   if (wire_type == WireFormatLite::WIRETYPE_LENGTH_DELIMITED) {
-    // EOF in the middle of delimited data for known non-repeated value.
     ExpectParseFailureForProto(
-        cat( tag(fieldnum, wire_type), varint(1) ));
+        cat( tag(fieldnum, wire_type), varint(1) ),
+        "PrematureEofInDelimitedDataForKnownNonRepeatedValue" + type_name);
 
-    // EOF in the middle of delimited data for known repeated value.
     ExpectParseFailureForProto(
-        cat( tag(rep_fieldnum, wire_type), varint(1) ));
+        cat( tag(rep_fieldnum, wire_type), varint(1) ),
+        "PrematureEofInDelimitedDataForKnownRepeatedValue" + type_name);
 
     // EOF in the middle of delimited data for unknown value.
     ExpectParseFailureForProto(
-        cat( tag(UNKNOWN_FIELD, wire_type), varint(1) ));
+        cat( tag(UNKNOWN_FIELD, wire_type), varint(1) ),
+        "PrematureEofInDelimitedDataForUnknownValue" + type_name);
 
-    if (type == WireFormatLite::TYPE_MESSAGE) {
+    if (type == FieldDescriptor::TYPE_MESSAGE) {
       // Submessage ends in the middle of a value.
       string incomplete_submsg =
           cat( tag(WireFormatLite::TYPE_INT32, WireFormatLite::WIRETYPE_VARINT),
@@ -269,42 +299,86 @@ void ConformanceTestSuite::TestPrematureEOFForType(
       ExpectHardParseFailureForProto(
           cat( tag(fieldnum, WireFormatLite::WIRETYPE_LENGTH_DELIMITED),
                varint(incomplete_submsg.size()),
-               incomplete_submsg ));
+               incomplete_submsg ),
+          "PrematureEofInSubmessageValue" + type_name);
     }
-  } else if (type != WireFormatLite::TYPE_GROUP) {
+  } else if (type != FieldDescriptor::TYPE_GROUP) {
     // Non-delimited, non-group: eligible for packing.
 
     // Packed region ends in the middle of a value.
     ExpectHardParseFailureForProto(
         cat( tag(rep_fieldnum, WireFormatLite::WIRETYPE_LENGTH_DELIMITED),
              varint(incomplete.size()),
-             incomplete ));
+             incomplete ),
+        "PrematureEofInPackedFieldValue" + type_name);
 
     // EOF in the middle of packed region.
     ExpectParseFailureForProto(
         cat( tag(rep_fieldnum, WireFormatLite::WIRETYPE_LENGTH_DELIMITED),
-             varint(1) ));
+             varint(1) ),
+        "PrematureEofInPackedField" + type_name);
   }
 }
 
-void ConformanceTestSuite::RunSuite(ConformanceTestRunner* runner,
+void ConformanceTestSuite::SetFailureList(const vector<string>& failure_list) {
+  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) {
+  if (set_to_check.empty()) {
+    return true;
+  } else {
+    StringAppendF(&output_, "\n");
+    StringAppendF(&output_, "ERROR: %s:\n", msg);
+    for (set<string>::const_iterator iter = set_to_check.begin();
+         iter != set_to_check.end(); ++iter) {
+      StringAppendF(&output_, "%s\n", iter->c_str());
+    }
+    return false;
+  }
+}
+
+bool ConformanceTestSuite::RunSuite(ConformanceTestRunner* runner,
                                     std::string* output) {
   runner_ = runner;
   output_.clear();
   successes_ = 0;
   failures_ = 0;
+  test_names_.clear();
+  unexpected_failing_tests_.clear();
+  unexpected_succeeding_tests_.clear();
 
   for (int i = 1; i <= FieldDescriptor::MAX_TYPE; i++) {
     if (i == FieldDescriptor::TYPE_GROUP) continue;
-    TestPrematureEOFForType(static_cast<WireFormatLite::FieldType>(i));
+    TestPrematureEOFForType(static_cast<FieldDescriptor::Type>(i));
   }
 
+  StringAppendF(&output_, "\n");
   StringAppendF(&output_,
                 "CONFORMANCE SUITE FINISHED: completed %d tests, %d successes, "
                 "%d failures.\n",
                 successes_ + failures_, successes_, failures_);
 
+  bool ok =
+      CheckSetEmpty(expected_to_fail_,
+                    "These tests were listed in the failure list, but they "
+                    "don't exist.  Remove them from the failure list") &&
+
+      CheckSetEmpty(unexpected_failing_tests_,
+                    "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") &&
+
+      CheckSetEmpty(unexpected_succeeding_tests_,
+                    "These tests succeeded, even though they were listed in "
+                    "the failure list.  Remove them from the failure list");
+
   output->assign(output_);
+
+  return ok;
 }
 
 }  // namespace protobuf

conformance/conformance_test.h

@@ -83,24 +83,49 @@ class ConformanceTestSuite {
  public:
   ConformanceTestSuite() : verbose_(false) {}
 
+  // 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);
+
   // Run all the conformance tests against the given test runner.
   // Test output will be stored in "output".
-  void RunSuite(ConformanceTestRunner* runner, std::string* output);
+  //
+  // Returns true if the set of failing tests was exactly the same as the
+  // failure list.  If SetFailureList() was not called, returns true if all
+  // tests passed.
+  bool RunSuite(ConformanceTestRunner* runner, std::string* output);
 
  private:
-  void ReportSuccess();
-  void ReportFailure(const char* fmt, ...);
+  void ReportSuccess(const std::string& test_name);
+  void ReportFailure(const std::string& test_name, const char* fmt, ...);
   void RunTest(const conformance::ConformanceRequest& request,
                conformance::ConformanceResponse* response);
-  void DoExpectParseFailureForProto(const std::string& proto, int line);
-  void TestPrematureEOFForType(
-      google::protobuf::internal::WireFormatLite::FieldType type);
-
+  void ExpectParseFailureForProto(const std::string& proto,
+                                  const std::string& test_name);
+  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);
   ConformanceTestRunner* runner_;
   int successes_;
   int failures_;
   bool verbose_;
   std::string output_;
+
+  // The set of test names that are expected to fail in this run, but haven't
+  // failed yet.
+  std::set<std::string> expected_to_fail_;
+
+  // The set of test names that have been run.  Used to ensure that there are no
+  // duplicate names in the suite.
+  std::set<std::string> test_names_;
+
+  // The set of tests that failed, but weren't expected to.
+  std::set<std::string> unexpected_failing_tests_;
+
+  // The set of tests that succeeded, but weren't expected to.
+  std::set<std::string> unexpected_succeeding_tests_;
 };
 
 }  // namespace protobuf

conformance/conformance_test_runner.cc

@@ -55,6 +55,8 @@
 
 #include <errno.h>
 #include <unistd.h>
+#include <fstream>
+#include <vector>
 
 #include "conformance.pb.h"
 #include "conformance_test.h"
@@ -62,6 +64,8 @@
 using conformance::ConformanceRequest;
 using conformance::ConformanceResponse;
 using google::protobuf::internal::scoped_array;
+using std::string;
+using std::vector;
 
 #define STRINGIFY(x) #x
 #define TOSTRING(x) STRINGIFY(x)
@@ -180,18 +184,67 @@ class ForkPipeRunner : public google::protobuf::ConformanceTestRunner {
   std::string executable_;
 };
 
+void UsageError() {
+  fprintf(stderr,
+          "Usage: conformance-test-runner [options] <test-program>\n");
+  fprintf(stderr, "\n");
+  fprintf(stderr, "Options:\n");
+  fprintf(stderr,
+          "  --failure_list <filename>   Use to specify list of tests\n");
+  fprintf(stderr,
+          "                              that are expected to fail.  File\n");
+  fprintf(stderr,
+          "                              should contain one test name per\n");
+  fprintf(stderr,
+          "                              line.  Use '#' for comments.\n");
+  exit(1);
+}
+
+void ParseFailureList(const char *filename, vector<string>* failure_list) {
+  std::ifstream infile(filename);
+  for (string line; getline(infile, line);) {
+    // Remove whitespace.
+    line.erase(std::remove_if(line.begin(), line.end(), ::isspace),
+               line.end());
+
+    // Remove comments.
+    line = line.substr(0, line.find("#"));
+
+    if (!line.empty()) {
+      failure_list->push_back(line);
+    }
+  }
+}
 
 int main(int argc, char *argv[]) {
-  if (argc < 2) {
-    fprintf(stderr, "Usage: conformance-test-runner <test-program>\n");
-    exit(1);
+  int arg = 1;
+  char *program;
+  vector<string> failure_list;
+
+  for (int arg = 1; arg < argc; ++arg) {
+    if (strcmp(argv[arg], "--failure_list") == 0) {
+      if (++arg == argc) UsageError();
+      ParseFailureList(argv[arg], &failure_list);
+    } else if (argv[arg][0] == '-') {
+      fprintf(stderr, "Unknown option: %s\n", argv[arg]);
+      UsageError();
+    } else {
+      if (arg != argc - 1) {
+        fprintf(stderr, "Too many arguments.\n");
+        UsageError();
+      }
+      program = argv[arg];
+    }
   }
 
-  ForkPipeRunner runner(argv[1]);
+  ForkPipeRunner runner(program);
   google::protobuf::ConformanceTestSuite suite;
+  suite.SetFailureList(failure_list);
 
   std::string output;
-  suite.RunSuite(&runner, &output);
+  bool ok = suite.RunSuite(&runner, &output);
 
   fwrite(output.c_str(), 1, output.size(), stderr);
+
+  return ok ? EXIT_SUCCESS : EXIT_FAILURE;
 }

conformance/failure_list_cpp.txt

@@ -0,0 +1,21 @@
+# This is the list of conformance tests that are known to fail for the C++
+# implementation right now.  These should be fixed.
+#
+# By listing them here we can keep tabs on which ones are failing and be sure
+# that we don't introduce regressions in other tests.
+#
+# TODO(haberman): insert links to corresponding bugs tracking the issue.
+# Should we use GitHub issues or the Google-internal bug tracker.
+
+PrematureEofBeforeKnownRepeatedValue.MESSAGE
+PrematureEofInDelimitedDataForKnownNonRepeatedValue.MESSAGE
+PrematureEofInDelimitedDataForKnownRepeatedValue.MESSAGE
+PrematureEofInPackedField.BOOL
+PrematureEofInPackedField.ENUM
+PrematureEofInPackedField.INT32
+PrematureEofInPackedField.INT64
+PrematureEofInPackedField.SINT32
+PrematureEofInPackedField.SINT64
+PrematureEofInPackedField.UINT32
+PrematureEofInPackedField.UINT64
+PrematureEofInsideKnownRepeatedValue.MESSAGE