Merge pull request #588 from haberman/conformance-json

Added support for JSON and valid input to conformance tests.

conformance/ConformanceJava.java

@@ -54,7 +54,7 @@ class ConformanceJava {
         break;
       }
       case JSON_PAYLOAD: {
-        return Conformance.ConformanceResponse.newBuilder().setRuntimeError("JSON not yet supported.").build();
+        return Conformance.ConformanceResponse.newBuilder().setSkipped("JSON not yet supported.").build();
       }
       case PAYLOAD_NOT_SET: {
         throw new RuntimeException("Request didn't have payload.");
@@ -65,7 +65,7 @@ class ConformanceJava {
       }
     }
 
-    switch (request.getRequestedOutput()) {
+    switch (request.getRequestedOutputFormat()) {
       case UNSPECIFIED:
         throw new RuntimeException("Unspecified output format.");
 
@@ -73,7 +73,7 @@ class ConformanceJava {
         return Conformance.ConformanceResponse.newBuilder().setProtobufPayload(testMessage.toByteString()).build();
 
       case JSON:
-        return Conformance.ConformanceResponse.newBuilder().setRuntimeError("JSON not yet supported.").build();
+        return Conformance.ConformanceResponse.newBuilder().setSkipped("JSON not yet supported.").build();
 
       default: {
         throw new RuntimeException("Unexpected request output.");

conformance/conformance.proto

@@ -51,6 +51,12 @@ option java_package = "com.google.protobuf.conformance";
 //   - running as a sub-process may be more tricky in unusual environments like
 //     iOS apps, where fork/stdin/stdout are not available.
 
+enum WireFormat {
+  UNSPECIFIED = 0;
+  PROTOBUF = 1;
+  JSON = 2;
+}
+
 // Represents a single test case's input.  The testee should:
 //
 //   1. parse this proto (which should always succeed)
@@ -64,14 +70,8 @@ message ConformanceRequest {
     string json_payload = 2;
   }
 
-  enum RequestedOutput {
-    UNSPECIFIED = 0;
-    PROTOBUF = 1;
-    JSON = 2;
-  }
-
   // Which format should the testee serialize its message to?
-  RequestedOutput requested_output = 3;
+  WireFormat requested_output_format = 3;
 }
 
 // Represents a single test case's output.
@@ -96,6 +96,10 @@ message ConformanceResponse {
     // If the input was successfully parsed and the requested output was JSON,
     // serialize to JSON and set it in this field.
     string json_payload = 4;
+
+    // For when the testee skipped the test, likely because a certain feature
+    // wasn't supported, like JSON input/output.
+    string skipped = 5;
   }
 }
 

conformance/conformance_cpp.cc

@@ -33,14 +33,33 @@
 #include <unistd.h>
 
 #include "conformance.pb.h"
+#include <google/protobuf/util/json_util.h>
+#include <google/protobuf/util/type_resolver_util.h>
 
-using std::string;
 using conformance::ConformanceRequest;
 using conformance::ConformanceResponse;
 using conformance::TestAllTypes;
+using google::protobuf::Descriptor;
+using google::protobuf::DescriptorPool;
+using google::protobuf::internal::scoped_ptr;
+using google::protobuf::util::BinaryToJsonString;
+using google::protobuf::util::JsonToBinaryString;
+using google::protobuf::util::NewTypeResolverForDescriptorPool;
+using google::protobuf::util::Status;
+using google::protobuf::util::TypeResolver;
+using std::string;
+
+static const char kTypeUrlPrefix[] = "type.googleapis.com";
+
+static string GetTypeUrl(const Descriptor* message) {
+  return string(kTypeUrlPrefix) + "/" + message->full_name();
+}
 
 int test_count = 0;
 bool verbose = false;
+TypeResolver* type_resolver;
+string* type_url;
+
 
 bool CheckedRead(int fd, void *buf, size_t len) {
   size_t ofs = 0;
@@ -79,28 +98,44 @@ void DoTest(const ConformanceRequest& request, ConformanceResponse* response) {
       }
       break;
 
-    case ConformanceRequest::kJsonPayload:
+    case ConformanceRequest::kJsonPayload: {
-      response->set_runtime_error("JSON input is not yet supported.");
+      string proto_binary;
+      Status status = JsonToBinaryString(type_resolver, *type_url,
+                                         request.json_payload(), &proto_binary);
+      if (!status.ok()) {
+        response->set_parse_error(string("Parse error: ") +
+                                  status.error_message().as_string());
+        return;
+      }
+
+      GOOGLE_CHECK(test_message.ParseFromString(proto_binary));
       break;
+    }
 
     case ConformanceRequest::PAYLOAD_NOT_SET:
       GOOGLE_LOG(FATAL) << "Request didn't have payload.";
       break;
   }
 
-  switch (request.requested_output()) {
+  switch (request.requested_output_format()) {
-    case ConformanceRequest::UNSPECIFIED:
+    case conformance::UNSPECIFIED:
       GOOGLE_LOG(FATAL) << "Unspecified output format";
       break;
 
-    case ConformanceRequest::PROTOBUF:
+    case conformance::PROTOBUF:
-      test_message.SerializeToString(response->mutable_protobuf_payload());
+      GOOGLE_CHECK(
+          test_message.SerializeToString(response->mutable_protobuf_payload()));
       break;
 
-    case ConformanceRequest::JSON:
+    case conformance::JSON: {
-      response->set_runtime_error("JSON output is not yet supported.");
+      string proto_binary;
+      GOOGLE_CHECK(test_message.SerializeToString(&proto_binary));
+      Status status = BinaryToJsonString(type_resolver, *type_url, proto_binary,
+                                         response->mutable_json_payload());
+      GOOGLE_CHECK(status.ok());
       break;
     }
+  }
 }
 
 bool DoTestIo() {
@@ -146,6 +181,9 @@ bool DoTestIo() {
 }
 
 int main() {
+  type_resolver = NewTypeResolverForDescriptorPool(
+      kTypeUrlPrefix, DescriptorPool::generated_pool());
+  type_url = new string(GetTypeUrl(TestAllTypes::descriptor()));
   while (1) {
     if (!DoTestIo()) {
       fprintf(stderr, "conformance-cpp: received EOF from test runner "

conformance/conformance_test.cc

@@ -35,18 +35,34 @@
 #include "conformance_test.h"
 #include <google/protobuf/stubs/common.h>
 #include <google/protobuf/stubs/stringprintf.h>
+#include <google/protobuf/text_format.h>
+#include <google/protobuf/util/json_util.h>
+#include <google/protobuf/util/message_differencer.h>
+#include <google/protobuf/util/type_resolver_util.h>
 #include <google/protobuf/wire_format_lite.h>
 
 using conformance::ConformanceRequest;
 using conformance::ConformanceResponse;
 using conformance::TestAllTypes;
+using conformance::WireFormat;
 using google::protobuf::Descriptor;
 using google::protobuf::FieldDescriptor;
 using google::protobuf::internal::WireFormatLite;
+using google::protobuf::TextFormat;
+using google::protobuf::util::JsonToBinaryString;
+using google::protobuf::util::MessageDifferencer;
+using google::protobuf::util::NewTypeResolverForDescriptorPool;
+using google::protobuf::util::Status;
 using std::string;
 
 namespace {
 
+static const char kTypeUrlPrefix[] = "type.googleapis.com";
+
+static string GetTypeUrl(const Descriptor* message) {
+  return string(kTypeUrlPrefix) + "/" + message->full_name();
+}
+
 /* Routines for building arbitrary protos *************************************/
 
 // We would use CodedOutputStream except that we want more freedom to build
@@ -162,9 +178,13 @@ void ConformanceTestSuite::ReportSuccess(const string& test_name) {
 }
 
 void ConformanceTestSuite::ReportFailure(const string& test_name,
+                                         const ConformanceRequest& request,
+                                         const ConformanceResponse& response,
                                          const char* fmt, ...) {
   if (expected_to_fail_.erase(test_name) == 1) {
-    StringAppendF(&output_, "FAILED AS EXPECTED, test=%s: ", test_name.c_str());
+    expected_failures_++;
+    if (!verbose_)
+      return;
   } else {
     StringAppendF(&output_, "ERROR, test=%s: ", test_name.c_str());
     unexpected_failing_tests_.insert(test_name);
@@ -173,7 +193,20 @@ void ConformanceTestSuite::ReportFailure(const string& test_name,
   va_start(args, fmt);
   StringAppendV(&output_, fmt, args);
   va_end(args);
-  failures_++;
+  StringAppendF(&output_, " request=%s, response=%s\n",
+                request.ShortDebugString().c_str(),
+                response.ShortDebugString().c_str());
+}
+
+void ConformanceTestSuite::ReportSkip(const string& test_name,
+                                      const ConformanceRequest& request,
+                                      const ConformanceResponse& response) {
+  if (verbose_) {
+    StringAppendF(&output_, "SKIPPED, test=%s request=%s, response=%s\n",
+                  test_name.c_str(), request.ShortDebugString().c_str(),
+                  response.ShortDebugString().c_str());
+  }
+  skipped_.insert(test_name);
 }
 
 void ConformanceTestSuite::RunTest(const string& test_name,
@@ -202,26 +235,117 @@ void ConformanceTestSuite::RunTest(const string& test_name,
   }
 }
 
+void ConformanceTestSuite::RunValidInputTest(
+    const string& test_name, const string& input, WireFormat input_format,
+    const string& equivalent_text_format, WireFormat requested_output) {
+  TestAllTypes reference_message;
+  GOOGLE_CHECK(
+      TextFormat::ParseFromString(equivalent_text_format, &reference_message));
+
+  ConformanceRequest request;
+  ConformanceResponse response;
+
+  switch (input_format) {
+    case conformance::PROTOBUF:
+      request.set_protobuf_payload(input);
+      break;
+
+    case conformance::JSON:
+      request.set_json_payload(input);
+      break;
+
+    case conformance::UNSPECIFIED:
+      GOOGLE_LOG(FATAL) << "Unspecified input format";
+
+  }
+
+  request.set_requested_output_format(requested_output);
+
+  RunTest(test_name, request, &response);
+
+  TestAllTypes test_message;
+
+  switch (response.result_case()) {
+    case ConformanceResponse::kParseError:
+    case ConformanceResponse::kRuntimeError:
+      ReportFailure(test_name, request, response,
+                    "Failed to parse valid JSON input.");
+      return;
+
+    case ConformanceResponse::kSkipped:
+      ReportSkip(test_name, request, response);
+      return;
+
+    case ConformanceResponse::kJsonPayload: {
+      if (requested_output != conformance::JSON) {
+        ReportFailure(
+            test_name, request, response,
+            "Test was asked for protobuf output but provided JSON instead.");
+        return;
+      }
+      string binary_protobuf;
+      Status status =
+          JsonToBinaryString(type_resolver_.get(), type_url_,
+                             response.json_payload(), &binary_protobuf);
+      if (!status.ok()) {
+        ReportFailure(test_name, request, response,
+                      "JSON output we received from test was unparseable.");
+        return;
+      }
+
+      GOOGLE_CHECK(test_message.ParseFromString(binary_protobuf));
+      break;
+    }
+
+    case ConformanceResponse::kProtobufPayload: {
+      if (requested_output != conformance::PROTOBUF) {
+        ReportFailure(
+            test_name, request, response,
+            "Test was asked for JSON output but provided protobuf instead.");
+        return;
+      }
+
+      if (!test_message.ParseFromString(response.protobuf_payload())) {
+        ReportFailure(test_name, request, response,
+                      "Protobuf output we received from test was unparseable.");
+        return;
+      }
+
+      break;
+    }
+  }
+
+  MessageDifferencer differencer;
+  string differences;
+  differencer.ReportDifferencesToString(&differences);
+
+  if (differencer.Equals(reference_message, test_message)) {
+    ReportSuccess(test_name);
+  } else {
+    ReportFailure(test_name, request, response,
+                  "Output was not equivalent to reference message: %s.",
+                  differences.c_str());
+  }
+}
+
 // 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);
+  string effective_test_name = "ProtobufInput." + test_name;
 
   // We don't expect output, but if the program erroneously accepts the protobuf
   // we let it send its response as this.  We must not leave it unspecified.
-  request.set_requested_output(ConformanceRequest::PROTOBUF);
+  request.set_requested_output_format(conformance::PROTOBUF);
 
-  RunTest(test_name, request, &response);
+  RunTest(effective_test_name, request, &response);
   if (response.result_case() == ConformanceResponse::kParseError) {
-    ReportSuccess(test_name);
+    ReportSuccess(effective_test_name);
   } else {
-    ReportFailure(test_name,
+    ReportFailure(effective_test_name, request, response,
-                  "Should have failed to parse, but didn't. Request: %s, "
+                  "Should have failed to parse, but didn't.");
-                  "response: %s\n",
-                  request.ShortDebugString().c_str(),
-                  response.ShortDebugString().c_str());
   }
 }
 
@@ -235,6 +359,16 @@ void ConformanceTestSuite::ExpectHardParseFailureForProto(
   return ExpectParseFailureForProto(proto, test_name);
 }
 
+void ConformanceTestSuite::RunValidJsonTest(
+    const string& test_name, const string& input_json,
+    const string& equivalent_text_format) {
+  RunValidInputTest("JsonInput." + test_name + ".JsonOutput", input_json,
+                    conformance::JSON, equivalent_text_format,
+                    conformance::PROTOBUF);
+  RunValidInputTest("JsonInput." + test_name + ".ProtobufOutput", input_json, conformance::JSON,
+                    equivalent_text_format, conformance::JSON);
+}
+
 void ConformanceTestSuite::TestPrematureEOFForType(FieldDescriptor::Type type) {
   // Incomplete values for each wire type.
   static const string incompletes[6] = {
@@ -333,11 +467,12 @@ bool ConformanceTestSuite::CheckSetEmpty(const set<string>& set_to_check,
     return true;
   } else {
     StringAppendF(&output_, "\n");
-    StringAppendF(&output_, "ERROR: %s:\n", msg);
+    StringAppendF(&output_, "%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());
+      StringAppendF(&output_, "  %s\n", iter->c_str());
     }
+    StringAppendF(&output_, "\n");
     return false;
   }
 }
@@ -345,23 +480,25 @@ bool ConformanceTestSuite::CheckSetEmpty(const set<string>& set_to_check,
 bool ConformanceTestSuite::RunSuite(ConformanceTestRunner* runner,
                                     std::string* output) {
   runner_ = runner;
-  output_.clear();
   successes_ = 0;
-  failures_ = 0;
+  expected_failures_ = 0;
+  skipped_.clear();
   test_names_.clear();
   unexpected_failing_tests_.clear();
   unexpected_succeeding_tests_.clear();
+  type_resolver_.reset(NewTypeResolverForDescriptorPool(
+      kTypeUrlPrefix, DescriptorPool::generated_pool()));
+  type_url_ = GetTypeUrl(TestAllTypes::descriptor());
+
+  output_ = "\nCONFORMANCE TEST BEGIN ====================================\n\n";
 
   for (int i = 1; i <= FieldDescriptor::MAX_TYPE; i++) {
     if (i == FieldDescriptor::TYPE_GROUP) continue;
     TestPrematureEOFForType(static_cast<FieldDescriptor::Type>(i));
   }
 
-  StringAppendF(&output_, "\n");
+  RunValidJsonTest("HelloWorld", "{\"optionalString\":\"Hello, World!\"}",
-  StringAppendF(&output_,
+                   "optional_string: 'Hello, World!'");
-                "CONFORMANCE SUITE FINISHED: completed %d tests, %d successes, "
-                "%d failures.\n",
-                successes_ + failures_, successes_, failures_);
 
   bool ok =
       CheckSetEmpty(expected_to_fail_,
@@ -377,6 +514,17 @@ bool ConformanceTestSuite::RunSuite(ConformanceTestRunner* runner,
                     "These tests succeeded, even though they were listed in "
                     "the failure list.  Remove them from the failure list");
 
+  CheckSetEmpty(skipped_,
+                "These tests were skipped (probably because support for some "
+                "features is not implemented)");
+
+  StringAppendF(&output_,
+                "CONFORMANCE SUITE %s: %d successes, %d skipped, "
+                "%d expected failures, %d unexpected failures.\n",
+                ok ? "PASSED" : "FAILED", successes_, skipped_.size(),
+                expected_failures_, unexpected_failing_tests_.size());
+  StringAppendF(&output_, "\n");
+
   output->assign(output_);
 
   return ok;

conformance/conformance_test.h

@@ -39,6 +39,8 @@
 #define CONFORMANCE_CONFORMANCE_TEST_H
 
 #include <string>
+#include <google/protobuf/stubs/common.h>
+#include <google/protobuf/util/type_resolver.h>
 #include <google/protobuf/wire_format_lite.h>
 
 namespace conformance {
@@ -98,10 +100,22 @@ class ConformanceTestSuite {
 
  private:
   void ReportSuccess(const std::string& test_name);
-  void ReportFailure(const std::string& test_name, const char* fmt, ...);
+  void ReportFailure(const string& test_name,
+                     const conformance::ConformanceRequest& request,
+                     const conformance::ConformanceResponse& response,
+                     const char* fmt, ...);
+  void ReportSkip(const string& test_name,
+                  const conformance::ConformanceRequest& request,
+                  const conformance::ConformanceResponse& response);
   void RunTest(const std::string& test_name,
                const conformance::ConformanceRequest& request,
                conformance::ConformanceResponse* response);
+  void RunValidInputTest(const string& test_name, const string& input,
+                         conformance::WireFormat input_format,
+                         const string& equivalent_text_format,
+                         conformance::WireFormat requested_output);
+  void RunValidJsonTest(const string& test_name, const string& input_json,
+                        const string& equivalent_text_format);
   void ExpectParseFailureForProto(const std::string& proto,
                                   const std::string& test_name);
   void ExpectHardParseFailureForProto(const std::string& proto,
@@ -110,7 +124,7 @@ class ConformanceTestSuite {
   bool CheckSetEmpty(const set<string>& set_to_check, const char* msg);
   ConformanceTestRunner* runner_;
   int successes_;
-  int failures_;
+  int expected_failures_;
   bool verbose_;
   std::string output_;
 
@@ -127,6 +141,13 @@ class ConformanceTestSuite {
 
   // The set of tests that succeeded, but weren't expected to.
   std::set<std::string> unexpected_succeeding_tests_;
+
+  // The set of tests that the testee opted out of;
+  std::set<std::string> skipped_;
+
+  google::protobuf::internal::scoped_ptr<google::protobuf::util::TypeResolver>
+      type_resolver_;
+  std::string type_url_;
 };
 
 }  // namespace protobuf

conformance/failure_list_cpp.txt

@@ -7,15 +7,15 @@
 # TODO(haberman): insert links to corresponding bugs tracking the issue.
 # Should we use GitHub issues or the Google-internal bug tracker?
 
-PrematureEofBeforeKnownRepeatedValue.MESSAGE
+ProtobufInput.PrematureEofBeforeKnownRepeatedValue.MESSAGE
-PrematureEofInDelimitedDataForKnownNonRepeatedValue.MESSAGE
+ProtobufInput.PrematureEofInDelimitedDataForKnownNonRepeatedValue.MESSAGE
-PrematureEofInDelimitedDataForKnownRepeatedValue.MESSAGE
+ProtobufInput.PrematureEofInDelimitedDataForKnownRepeatedValue.MESSAGE
-PrematureEofInPackedField.BOOL
+ProtobufInput.PrematureEofInPackedField.BOOL
-PrematureEofInPackedField.ENUM
+ProtobufInput.PrematureEofInPackedField.ENUM
-PrematureEofInPackedField.INT32
+ProtobufInput.PrematureEofInPackedField.INT32
-PrematureEofInPackedField.INT64
+ProtobufInput.PrematureEofInPackedField.INT64
-PrematureEofInPackedField.SINT32
+ProtobufInput.PrematureEofInPackedField.SINT32
-PrematureEofInPackedField.SINT64
+ProtobufInput.PrematureEofInPackedField.SINT64
-PrematureEofInPackedField.UINT32
+ProtobufInput.PrematureEofInPackedField.UINT32
-PrematureEofInPackedField.UINT64
+ProtobufInput.PrematureEofInPackedField.UINT64
-PrematureEofInsideKnownRepeatedValue.MESSAGE
+ProtobufInput.PrematureEofInsideKnownRepeatedValue.MESSAGE