Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement respond_to? in RubyMessage #9677

Merged
merged 9 commits into from
Mar 28, 2022

Conversation

JasonLunn
Copy link
Contributor

@JasonLunn JasonLunn commented Mar 25, 2022

All synthetic method names implemented by method_missing return true. Add regression tests.

  • Fix null pointer exceptions and class cast exceptions in method_missing exposed by new unit tests.
  • Fix bug where multiple calls to clear_ on a oneof field would throw an exception.
  • Align behavior with CRuby implementation by throwing a RuntimeError rather than a NoMethodError when attempting to assign to a oneof.

Code cleanup: remove test code that was skipping several unit tests only on JRuby.

Fixes issue #9202.

…implemented by `method_missing` return true. Add regression tests.

Fix null pointer exceptions exposed by new unit tests.

Code cleanup: remove test code that was skipping several unit tests only on JRuby.

Fixes issue protocolbuffers#9202.
…t respond_to? does not depend on whether the oneof is currently cleared.
…t respond_to? does not depend on whether the oneof is currently cleared.

Code cleanup: reenable more tests on JRuby.
Code cleanup: Eliminate noisy warnings when running tests by
1) removing or using unused variables
2) wrapping ambiguous parameters in parentheses
Copy link
Contributor

@perezd perezd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@@ -487,7 +487,7 @@ def test_shift
def test_shuffle!
m = TestMessage.new
m.repeated_string += %w(foo bar baz)
orig_repeated_string = m.repeated_string.clone
# orig_repeated_string = m.repeated_string.clone
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be uncommented?

assert_equal -1.01, m.repeated_float.pop.round(2)
assert_equal -1.0000000000002, m.repeated_double.pop
assert_equal -1.0000000000001, m.repeated_double.pop
assert_equal( -1.02, m.repeated_float.pop.round(2) )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all of these edits seem irrelevant to the goal of the PR, I'd revert for noise sake.

}
}
boolean includePrivate = false;
if ( args.length == 2 ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rm whitespace within ( )

@JasonLunn JasonLunn merged commit 8e7f936 into protocolbuffers:master Mar 28, 2022
@JasonLunn JasonLunn deleted the fix_9202 branch March 28, 2022 22:44
JasonLunn added a commit to JasonLunn/protobuf that referenced this pull request Mar 31, 2022
All synthetic methods implemented by `method_missing` are now supported by `respond_to?`.

Fixes issue protocolbuffers#9202.

* Fix null pointer exceptions exposed by new regression tests.
* Fix clear_ on oneofs so that it is safe to call repeatedly and so that respond_to? does not depend on whether the oneof is currently cleared.
* Code cleanup: reenable more tests on JRuby.
* Align JRuby behavior with CRuby by throwing a RuntimeError when attempting to assign to a oneof.

(cherry picked from commit 8e7f936)
mkruskal-google added a commit that referenced this pull request Apr 7, 2022
* Fix NPE during encoding and add regression test for issue 9507.

(cherry picked from commit 58e320a)

* Implement `respond_to?` in RubyMessage (#9677)

All synthetic methods implemented by `method_missing` are now supported by `respond_to?`.

Fixes issue #9202.

* Fix null pointer exceptions exposed by new regression tests.
* Fix clear_ on oneofs so that it is safe to call repeatedly and so that respond_to? does not depend on whether the oneof is currently cleared.
* Code cleanup: reenable more tests on JRuby.
* Align JRuby behavior with CRuby by throwing a RuntimeError when attempting to assign to a oneof.

(cherry picked from commit 8e7f936)

* Update protobuf version

* Merge pull request #9727 from mlocati/build-packaged-php-extension

Fix building packaged PHP extension

(cherry picked from commit 7f9901c)

* Update protobuf version

* Update changelogs for 3.20.1-rc1

Co-authored-by: Jason Lunn <[email protected]>
Co-authored-by: Jorg Brown <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jruby Issues unique to the JRuby interpreter ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants