Merge pull request #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

+#!/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

+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

ruby/lib/google/protobuf.rb

@@ -31,6 +31,15 @@
 # require mixins before we hook them into the java & c code
 require 'google/protobuf/message_exts'
 
+# We define these before requiring the platform-specific modules.
+# That way the module init can grab references to these.
+module Google
+  module Protobuf
+    class Error < StandardError; end
+    class ParseError < Error; end
+  end
+end
+
 if RUBY_PLATFORM == "java"
   require 'json'
   require 'google/protobuf_java'

ruby/travis-test.sh

@@ -5,11 +5,22 @@ set -e
 
 test_version() {
   version=$1
-  bash --login -c \
-    "rvm install $version && rvm use $version && \
-     which ruby && \
-     gem install bundler && bundle && \
-     rake test"
+  if [ "$version" == "jruby" ] ; then
+    # No conformance tests yet -- JRuby is too broken to run them.
+    bash --login -c \
+      "rvm install $version && rvm use $version && \
+       which ruby && \
+       gem install bundler && bundle && \
+       rake test"
+  else
+    bash --login -c \
+      "rvm install $version && rvm use $version && \
+       which ruby && \
+       gem install bundler && bundle && \
+       rake test && \
+       cd ../conformance && \
+       make test_ruby"
+  fi
 }
 
 test_version $1

src/google/protobuf/compiler/ruby/ruby_generated_code.rb

@@ -13,7 +13,7 @@ Google::Protobuf::DescriptorPool.generated_pool.build do
     optional :optional_double, :double, 6
     optional :optional_float, :float, 7
     optional :optional_string, :string, 8
-    optional :optional_bytes, :string, 9
+    optional :optional_bytes, :bytes, 9
     optional :optional_enum, :enum, 10, "A.B.C.TestEnum"
     optional :optional_msg, :message, 11, "A.B.C.TestMessage"
     repeated :repeated_int32, :int32, 21
@@ -24,7 +24,7 @@ Google::Protobuf::DescriptorPool.generated_pool.build do
     repeated :repeated_double, :double, 26
     repeated :repeated_float, :float, 27
     repeated :repeated_string, :string, 28
-    repeated :repeated_bytes, :string, 29
+    repeated :repeated_bytes, :bytes, 29
     repeated :repeated_enum, :enum, 30, "A.B.C.TestEnum"
     repeated :repeated_msg, :message, 31, "A.B.C.TestMessage"
     map :map_int32_string, :int32, :string, 61
@@ -47,7 +47,7 @@ Google::Protobuf::DescriptorPool.generated_pool.build do
       optional :oneof_double, :double, 46
       optional :oneof_float, :float, 47
       optional :oneof_string, :string, 48
-      optional :oneof_bytes, :string, 49
+      optional :oneof_bytes, :bytes, 49
       optional :oneof_enum, :enum, 50, "A.B.C.TestEnum"
       optional :oneof_msg, :message, 51, "A.B.C.TestMessage"
     end

src/google/protobuf/compiler/ruby/ruby_generator.cc

@@ -47,7 +47,7 @@ namespace compiler {
 namespace ruby {
 
 // Forward decls.
-std::string IntToString(uint32 value);
+std::string IntToString(int32 value);
 std::string StripDotProto(const std::string& proto_file);
 std::string LabelForField(google::protobuf::FieldDescriptor* field);
 std::string TypeName(google::protobuf::FieldDescriptor* field);
@@ -64,7 +64,7 @@ void GenerateEnumAssignment(
     const google::protobuf::EnumDescriptor* en,
     google::protobuf::io::Printer* printer);
 
-std::string IntToString(uint32 value) {
+std::string IntToString(int32 value) {
   std::ostringstream os;
   os << value;
   return os.str();
@@ -85,17 +85,25 @@ std::string LabelForField(const google::protobuf::FieldDescriptor* field) {
 }
 
 std::string TypeName(const google::protobuf::FieldDescriptor* field) {
-  switch (field->cpp_type()) {
-    case FieldDescriptor::CPPTYPE_INT32: return "int32";
-    case FieldDescriptor::CPPTYPE_INT64: return "int64";
-    case FieldDescriptor::CPPTYPE_UINT32: return "uint32";
-    case FieldDescriptor::CPPTYPE_UINT64: return "uint64";
-    case FieldDescriptor::CPPTYPE_DOUBLE: return "double";
-    case FieldDescriptor::CPPTYPE_FLOAT: return "float";
-    case FieldDescriptor::CPPTYPE_BOOL: return "bool";
-    case FieldDescriptor::CPPTYPE_ENUM: return "enum";
-    case FieldDescriptor::CPPTYPE_STRING: return "string";
-    case FieldDescriptor::CPPTYPE_MESSAGE: return "message";
+  switch (field->type()) {
+    case FieldDescriptor::TYPE_INT32: return "int32";
+    case FieldDescriptor::TYPE_INT64: return "int64";
+    case FieldDescriptor::TYPE_UINT32: return "uint32";
+    case FieldDescriptor::TYPE_UINT64: return "uint64";
+    case FieldDescriptor::TYPE_SINT32: return "sint32";
+    case FieldDescriptor::TYPE_SINT64: return "sint64";
+    case FieldDescriptor::TYPE_FIXED32: return "fixed32";
+    case FieldDescriptor::TYPE_FIXED64: return "fixed64";
+    case FieldDescriptor::TYPE_SFIXED32: return "sfixed32";
+    case FieldDescriptor::TYPE_SFIXED64: return "sfixed64";
+    case FieldDescriptor::TYPE_DOUBLE: return "double";
+    case FieldDescriptor::TYPE_FLOAT: return "float";
+    case FieldDescriptor::TYPE_BOOL: return "bool";
+    case FieldDescriptor::TYPE_ENUM: return "enum";
+    case FieldDescriptor::TYPE_STRING: return "string";
+    case FieldDescriptor::TYPE_BYTES: return "bytes";
+    case FieldDescriptor::TYPE_MESSAGE: return "message";
+    case FieldDescriptor::TYPE_GROUP: return "group";
     default: assert(false); return "";
   }
 }

travis.sh

@@ -8,10 +8,16 @@
 # .travis.yml uses matrix.exclude to block the cases where app-get can't be
 # use to install things.
 
-build_cpp() {
+# For when some other test needs the C++ main build, including protoc and
+# libprotobuf.
+internal_build_cpp() {
   ./autogen.sh
   ./configure
   make -j2
+}
+
+build_cpp() {
+  internal_build_cpp
   make check -j2
   cd conformance && make test_cpp && cd ..
 }
@@ -62,18 +68,14 @@ use_java() {
 
 build_java() {
   # Java build needs `protoc`.
-  ./autogen.sh
-  ./configure
-  make -j2
+  internal_build_cpp
   cd java && mvn test && cd ..
   cd conformance && make test_java && cd ..
 }
 
 build_javanano() {
   # Java build needs `protoc`.
-  ./autogen.sh
-  ./configure
-  make -j2
+  internal_build_cpp
   cd javanano && mvn test && cd ..
 }
 
@@ -104,9 +106,7 @@ build_javanano_oracle7() {
 }
 
 build_python() {
-  ./autogen.sh
-  ./configure
-  make -j2
+  internal_build_cpp
   cd python
   python setup.py build
   python setup.py test
@@ -116,9 +116,7 @@ build_python() {
 }
 
 build_python_cpp() {
-  ./autogen.sh
-  ./configure
-  make -j2
+  internal_build_cpp
   export   LD_LIBRARY_PATH=../src/.libs # for Linux
   export DYLD_LIBRARY_PATH=../src/.libs # for OS X
   cd python
@@ -130,18 +128,23 @@ build_python_cpp() {
 }
 
 build_ruby19() {
+  internal_build_cpp  # For conformance tests.
   cd ruby && bash travis-test.sh ruby-1.9 && cd ..
 }
 build_ruby20() {
+  internal_build_cpp  # For conformance tests.
   cd ruby && bash travis-test.sh ruby-2.0 && cd ..
 }
 build_ruby21() {
+  internal_build_cpp  # For conformance tests.
   cd ruby && bash travis-test.sh ruby-2.1 && cd ..
 }
 build_ruby22() {
+  internal_build_cpp  # For conformance tests.
   cd ruby && bash travis-test.sh ruby-2.2 && cd ..
 }
 build_jruby() {
+  internal_build_cpp  # For conformance tests.
   cd ruby && bash travis-test.sh jruby && cd ..
 }