search

ruby-protobuf / protobuf

A pure ruby implementation of Google's Protocol Buffers
https://github.com/ruby-protobuf
MIT License
463 stars 101 forks source link

Add functional tests #346

Closed embark closed 7 years ago

embark commented 7 years ago

cc @film42 @liveh2o @nerdrew

Adding the google unittest proto for thorough functional testing of code generation

film42 commented 7 years ago

Very strange that the test is getting require 'protobuf' instead of require 'protobuf/message'.

embark commented 7 years ago

Ah whoops, @film42 that was from this commit https://github.com/ruby-protobuf/protobuf/pull/316/commits/6ec6ec7a4d2e533ef768415f2a833820364a30b0, which fixed a circular dependency warning

Seems like jruby failed at gem update bundler now? Transient perhaps?

film42 commented 7 years ago

Hmm.. we should probably change that back so the 3.7.0 release doesn't cause a every generated file to have a single line change, but we can add that to the 3.7.0 issue for later. Not a big deal IMO, though.

The jruby bundler issue is probably one-off. I thought I had a button to restart a job, would you mind kicking off that build again?

film42 commented 7 years ago

Derp! My travis session had expired. Just restarted the build.

film42 commented 7 years ago

Looks like the job failed with a Java::JavaLang::OutOfMemoryError.

film42 commented 7 years ago

Still running out of memory... that's really weird. Let me run this locally and see what I find.

embark commented 7 years ago

Hrm yeah, I'm running out of memory locally too. Looking into it :)

embark commented 7 years ago

Well, the heap problem was because the tag warnings code was doing (-53452..12589234).to_a. The sparse enum test case was the bug finder:

// Test an enum with large, unordered values.
enum TestSparseEnum {
  SPARSE_A = 123;
  SPARSE_B = 62374;
  SPARSE_C = 12589234;
  SPARSE_D = -15;
  SPARSE_E = -53452;
  SPARSE_F = 0;
  SPARSE_G = 2;
}

There's still a logical problem with jruby to look into though, it seems. Will look as soon as I can

film42 commented 7 years ago

I tried to replicate these failures locally using their respective jruby versions but couldn't make it happen. I'm re-running the tests now and it seems like jruby-9.1.7.0 passed, so I just kicked off jruby-1.7.19 and we'll see what happens 🤞 .

film42 commented 7 years ago

Nope. Jruby 1.7.19 is still failing.

film42 commented 7 years ago

Finally reproduced this on jruby-1.7.19 with the activesupport 4.2.7.1 (I had an older active support installed). It's re-raising a name error after calling safe_constantize. Uh... WAT?

film42 commented 7 years ago

The problem is that NameError#name in jruby 1.7 returns "Protobuf_unittest::ComplexOptionType1" in both 1.9 and 2.0 mode, and jruby 9k and MRI only return "ComplexOptionType1". This divergent behavior makes active support fail: https://github.com/rails/rails/blob/46bf9eea533f8a2fedbb0aaad12cae1c5a4b9612/activesupport/lib/active_support/inflector/methods.rb#L314-L315 Since jruby 1.7 is not supported anymore, I think it might be safe to drop support for jruby 1.7 in the next release.

@nerdrew @embark @zanker Are you guys using jruby 1.7 in production, or are you ok with dropping support?

embark commented 7 years ago

@film42 Thank you for the investigation!!

I'm happy to remove support for 1.7 -- that was really just the default that Travis was running for (backwards compatible?) reasons when you specify jruby without a version.

Pushed a new commit without it.