Merge pull request #460 from haberman/conformance-names

Conformance tests can now be excluded based on their names.

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_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, test=%s: ", test_name.c_str());
+  } else {
+    StringAppendF(&output_, "ERROR, test=%s: ", test_name.c_str());
+    unexpected_failing_tests_.insert(test_name);
+  }
   va_list args;
   va_start(args, fmt);
   StringAppendV(&output_, fmt, args);
@@ -156,8 +176,13 @@ void ConformanceTestSuite::ReportFailure(const char *fmt, ...) {
   failures_++;
 }
 
-void ConformanceTestSuite::RunTest(const ConformanceRequest& request,
+void ConformanceTestSuite::RunTest(const string& test_name,
+                                   const ConformanceRequest& request,
                                    ConformanceResponse* response) {
+  if (test_names_.insert(test_name).second == false) {
+    GOOGLE_LOG(FATAL) << "Duplicated test name: " << test_name;
+  }
+
   string serialized_request;
   string serialized_response;
   request.SerializeToString(&serialized_request);
@@ -170,14 +195,16 @@ void ConformanceTestSuite::RunTest(const ConformanceRequest& request,
   }
 
   if (verbose_) {
-    StringAppendF(&output_, "conformance test: request=%s, response=%s\n",
+    StringAppendF(&output_, "conformance test: name=%s, request=%s, response=%s\n",
+                  test_name.c_str(),
                   request.ShortDebugString().c_str(),
                   response->ShortDebugString().c_str());
   }
 }
 
-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_protobuf_payload(proto);
@@ -186,31 +213,29 @@ void ConformanceTestSuite::DoExpectParseFailureForProto(const string& proto,
   // we let it send its response as this.  We must not leave it unspecified.
   request.set_requested_output(ConformanceRequest::PROTOBUF);
 
-  RunTest(request, &response);
+  RunTest(test_name, 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 +248,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 +300,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,50 @@ 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 RunTest(const conformance::ConformanceRequest& request,
+  void ReportSuccess(const std::string& test_name);
+  void ReportFailure(const std::string& test_name, const char* fmt, ...);
+  void RunTest(const std::string& test_name,
+               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