Merge pull request protocolbuffers#604 from haberman/ruby-conformance

Added Ruby to conformance tests.

conformance/Makefile.am

@@ -22,7 +22,7 @@ conformance_cpp_CPPFLAGS = -I$(top_srcdir)/src
 if USE_EXTERNAL_PROTOC
 
 protoc_middleman: $(protoc_inputs)
-	$(PROTOC) -I$(srcdir) --cpp_out=. --java_out=. $^
+	$(PROTOC) -I$(srcdir) --cpp_out=. --java_out=. --ruby_out=. $^
 	touch protoc_middleman
 
 else
@@ -31,7 +31,7 @@ else
 # relative to srcdir, which may not be the same as the current directory when
 # building out-of-tree.
 protoc_middleman: $(top_srcdir)/src/protoc$(EXEEXT) $(protoc_inputs)
-	oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. --cpp_out=$$oldpwd --java_out=$$oldpwd $(protoc_inputs) )
+	oldpwd=`pwd` && ( cd $(srcdir) && $$oldpwd/../src/protoc$(EXEEXT) -I. --cpp_out=$$oldpwd --java_out=$$oldpwd --ruby_out=$$oldpwd $(protoc_inputs) )
 	touch protoc_middleman
 
 endif
@@ -61,3 +61,6 @@ test_cpp: protoc_middleman conformance-test-runner conformance-cpp
 
 test_java: protoc_middleman conformance-test-runner conformance-java
 	./conformance-test-runner ./conformance-java
+
+test_ruby: protoc_middleman conformance-test-runner
+	RUBYLIB=../ruby/lib:. ./conformance-test-runner --failure_list failure_list_ruby.txt ./conformance_ruby.rb

conformance/conformance_ruby.rb

@@ -0,0 +1,114 @@
+#!/usr/bin/env ruby
+#
+# Protocol Buffers - Google's data interchange format
+# Copyright 2008 Google Inc.  All rights reserved.
+# https://developers.google.com/protocol-buffers/
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+require 'conformance'
+
+$test_count = 0
+$verbose = false
+
+def do_test(request)
+  test_message = Conformance::TestAllTypes.new
+  response = Conformance::ConformanceResponse.new
+
+  begin
+    case request.payload
+    when :protobuf_payload
+      begin
+        test_message =
+          Conformance::TestAllTypes.decode(request.protobuf_payload)
+      rescue Google::Protobuf::ParseError => err
+        response.parse_error = err.message.encode('utf-8')
+        return response
+      end
+
+    when :json_payload
+      test_message = Conformance::TestAllTypes.decode_json(request.json_payload)
+
+    when nil
+      fail "Request didn't have payload"
+    end
+
+    case request.requested_output_format
+    when :UNSPECIFIED
+      fail 'Unspecified output format'
+
+    when :PROTOBUF
+      response.protobuf_payload = test_message.to_proto
+
+    when :JSON
+      response.json_payload = test_message.to_json
+    end
+  rescue StandardError => err
+    response.runtime_error = err.message.encode('utf-8')
+  end
+
+  response
+end
+
+# Returns true if the test ran successfully, false on legitimate EOF.
+# If EOF is encountered in an unexpected place, raises IOError.
+def do_test_io
+  length_bytes = STDIN.read(4)
+  return false if length_bytes.nil?
+
+  length = length_bytes.unpack('V').first
+  serialized_request = STDIN.read(length)
+  if serialized_request.nil? || serialized_request.length != length
+    fail IOError
+  end
+
+  request = Conformance::ConformanceRequest.decode(serialized_request)
+
+  response = do_test(request)
+
+  serialized_response = Conformance::ConformanceResponse.encode(response)
+  STDOUT.write([serialized_response.length].pack('V'))
+  STDOUT.write(serialized_response)
+  STDOUT.flush
+
+  if $verbose
+    STDERR.puts("conformance-cpp: request={request.to_json}, " \
+                                 "response={response.to_json}\n")
+  end
+
+  $test_count += 1
+
+  true
+end
+
+loop do
+  unless do_test_io
+    STDERR.puts('conformance-cpp: received EOF from test runner ' \
+                "after #{$test_count} tests, exiting")
+    break
+  end
+end

conformance/conformance_test.h

@@ -85,6 +85,8 @@ class ConformanceTestSuite {
  public:
   ConformanceTestSuite() : verbose_(false) {}
 
+  void SetVerbose(bool verbose) { verbose_ = verbose; }
+
   // 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.

conformance/conformance_test_runner.cc

@@ -219,12 +219,16 @@ void ParseFailureList(const char *filename, vector<string>* failure_list) {
 int main(int argc, char *argv[]) {
   int arg = 1;
   char *program;
-  vector<string> failure_list;
+  google::protobuf::ConformanceTestSuite suite;
 
   for (int arg = 1; arg < argc; ++arg) {
     if (strcmp(argv[arg], "--failure_list") == 0) {
       if (++arg == argc) UsageError();
+      vector<string> failure_list;
       ParseFailureList(argv[arg], &failure_list);
+      suite.SetFailureList(failure_list);
+    } else if (strcmp(argv[arg], "--verbose") == 0) {
+      suite.SetVerbose(true);
     } else if (argv[arg][0] == '-') {
       fprintf(stderr, "Unknown option: %s\n", argv[arg]);
       UsageError();
@@ -238,8 +242,6 @@ int main(int argc, char *argv[]) {
   }
 
   ForkPipeRunner runner(program);
-  google::protobuf::ConformanceTestSuite suite;
-  suite.SetFailureList(failure_list);
 
   std::string output;
   bool ok = suite.RunSuite(&runner, &output);

conformance/failure_list_ruby.txt

@@ -0,0 +1,17 @@
+JsonInput.HelloWorld.JsonOutput
+JsonInput.HelloWorld.ProtobufOutput
+ProtobufInput.PrematureEofBeforeUnknownValue.DOUBLE
+ProtobufInput.PrematureEofBeforeUnknownValue.FIXED32
+ProtobufInput.PrematureEofBeforeUnknownValue.FIXED64
+ProtobufInput.PrematureEofBeforeUnknownValue.FLOAT
+ProtobufInput.PrematureEofBeforeUnknownValue.SFIXED32
+ProtobufInput.PrematureEofBeforeUnknownValue.SFIXED64
+ProtobufInput.PrematureEofInDelimitedDataForUnknownValue.BYTES
+ProtobufInput.PrematureEofInDelimitedDataForUnknownValue.MESSAGE
+ProtobufInput.PrematureEofInDelimitedDataForUnknownValue.STRING
+ProtobufInput.PrematureEofInsideUnknownValue.DOUBLE
+ProtobufInput.PrematureEofInsideUnknownValue.FIXED32
+ProtobufInput.PrematureEofInsideUnknownValue.FIXED64
+ProtobufInput.PrematureEofInsideUnknownValue.FLOAT
+ProtobufInput.PrematureEofInsideUnknownValue.SFIXED32
+ProtobufInput.PrematureEofInsideUnknownValue.SFIXED64

ruby/ext/google/protobuf_c/defs.c

@@ -548,7 +548,7 @@ upb_fieldtype_t ruby_to_fieldtype(VALUE type) {
 
 #define CONVERT(upb, ruby)                                           \
   if (SYM2ID(type) == rb_intern( # ruby )) {                         \
-    return UPB_TYPE_ ## upb;                                          \
+    return UPB_TYPE_ ## upb;                                         \
   }
 
   CONVERT(FLOAT, float);
@@ -589,6 +589,68 @@ VALUE fieldtype_to_ruby(upb_fieldtype_t type) {
   return Qnil;
 }
 
+upb_descriptortype_t ruby_to_descriptortype(VALUE type) {
+  if (TYPE(type) != T_SYMBOL) {
+    rb_raise(rb_eArgError, "Expected symbol for field type.");
+  }
+
+#define CONVERT(upb, ruby)                                           \
+  if (SYM2ID(type) == rb_intern( # ruby )) {                         \
+    return UPB_DESCRIPTOR_TYPE_ ## upb;                              \
+  }
+
+  CONVERT(FLOAT, float);
+  CONVERT(DOUBLE, double);
+  CONVERT(BOOL, bool);
+  CONVERT(STRING, string);
+  CONVERT(BYTES, bytes);
+  CONVERT(MESSAGE, message);
+  CONVERT(GROUP, group);
+  CONVERT(ENUM, enum);
+  CONVERT(INT32, int32);
+  CONVERT(INT64, int64);
+  CONVERT(UINT32, uint32);
+  CONVERT(UINT64, uint64);
+  CONVERT(SINT32, sint32);
+  CONVERT(SINT64, sint64);
+  CONVERT(FIXED32, fixed32);
+  CONVERT(FIXED64, fixed64);
+  CONVERT(SFIXED32, sfixed32);
+  CONVERT(SFIXED64, sfixed64);
+
+#undef CONVERT
+
+  rb_raise(rb_eArgError, "Unknown field type.");
+  return 0;
+}
+
+VALUE descriptortype_to_ruby(upb_descriptortype_t type) {
+  switch (type) {
+#define CONVERT(upb, ruby)                                           \
+    case UPB_DESCRIPTOR_TYPE_ ## upb : return ID2SYM(rb_intern( # ruby ));
+    CONVERT(FLOAT, float);
+    CONVERT(DOUBLE, double);
+    CONVERT(BOOL, bool);
+    CONVERT(STRING, string);
+    CONVERT(BYTES, bytes);
+    CONVERT(MESSAGE, message);
+    CONVERT(GROUP, group);
+    CONVERT(ENUM, enum);
+    CONVERT(INT32, int32);
+    CONVERT(INT64, int64);
+    CONVERT(UINT32, uint32);
+    CONVERT(UINT64, uint64);
+    CONVERT(SINT32, sint32);
+    CONVERT(SINT64, sint64);
+    CONVERT(FIXED32, fixed32);
+    CONVERT(FIXED64, fixed64);
+    CONVERT(SFIXED32, sfixed32);
+    CONVERT(SFIXED64, sfixed64);
+#undef CONVERT
+  }
+  return Qnil;
+}
+
 /*
  * call-seq:
  *     FieldDescriptor.type => type
@@ -604,7 +666,7 @@ VALUE FieldDescriptor_type(VALUE _self) {
   if (!upb_fielddef_typeisset(self->fielddef)) {
     return Qnil;
   }
-  return fieldtype_to_ruby(upb_fielddef_type(self->fielddef));
+  return descriptortype_to_ruby(upb_fielddef_descriptortype(self->fielddef));
 }
 
 /*
@@ -617,7 +679,7 @@ VALUE FieldDescriptor_type(VALUE _self) {
 VALUE FieldDescriptor_type_set(VALUE _self, VALUE type) {
   DEFINE_SELF(FieldDescriptor, self, _self);
   upb_fielddef* mut_def = check_field_notfrozen(self->fielddef);
-  upb_fielddef_settype(mut_def, ruby_to_fieldtype(type));
+  upb_fielddef_setdescriptortype(mut_def, ruby_to_descriptortype(type));
   return Qnil;
 }
 

ruby/ext/google/protobuf_c/encode_decode.c

@@ -656,8 +656,10 @@ static bool env_error_func(void* ud, const upb_status* status) {
   // Free the env -- rb_raise will longjmp up the stack past the encode/decode
   // function so it would not otherwise have been freed.
   stackenv_uninit(se);
-  rb_raise(rb_eRuntimeError, se->ruby_error_template,
-           upb_status_errmsg(status));
+
+  // TODO(haberman): have a way to verify that this is actually a parse error,
+  // instead of just throwing "parse error" unconditionally.
+  rb_raise(cParseError, se->ruby_error_template, upb_status_errmsg(status));
   // Never reached: rb_raise() always longjmp()s up the stack, past all of our
   // code, back to Ruby.
   return false;

ruby/ext/google/protobuf_c/protobuf.c

@@ -39,6 +39,9 @@
 // Ruby integers) to MessageDef/EnumDef instances (as Ruby values).
 VALUE upb_def_to_ruby_obj_map;
 
+VALUE cError;
+VALUE cParseError;
+
 void add_def_obj(const void* def, VALUE value) {
   rb_hash_aset(upb_def_to_ruby_obj_map, ULL2NUM((intptr_t)def), value);
 }
@@ -96,6 +99,9 @@ void Init_protobuf_c() {
   RepeatedField_register(protobuf);
   Map_register(protobuf);
 
+  cError = rb_const_get(protobuf, rb_intern("Error"));
+  cParseError = rb_const_get(protobuf, rb_intern("ParseError"));
+
   rb_define_singleton_method(protobuf, "deep_copy",
                              Google_Protobuf_deep_copy, 1);
 

ruby/ext/google/protobuf_c/protobuf.h

@@ -161,6 +161,9 @@ extern VALUE cOneofBuilderContext;
 extern VALUE cEnumBuilderContext;
 extern VALUE cBuilder;
 
+extern VALUE cError;
+extern VALUE cParseError;
+
 // We forward-declare all of the Ruby method implementations here because we
 // sometimes call the methods directly across .c files, rather than going
 // through Ruby's method dispatching (e.g. during message parse). It's cleaner