In Ruby, support each without block on repeated fields
Does what it says on the tin. ~Much to my pleasant surprise, this test currently passes (at least, it does on my machine)!~
Fixes https://github.com/protocolbuffers/protobuf/issues/5999
Oh, interesting. The test isn't running because it's out of testing scope -- looks like an adjacent test is also out of scope, which is where I went astray. I think this test currently fails if you make it actually run:
https://github.com/protocolbuffers/protobuf/pull/9677/files#diff-e247eaaa827af2292a6e18f804080f5d57c734e965436bbaccf8a3a76bb4a940R717
cc @JasonLunn let me know if I'm wildly off-base here, totally possible
Hi shaldengeki, do you plan to make these tests run (and fix them if they need fixing) in this PR, or in a separte one? Should we convert this one to draft in the meantime? Thanks for clarification :)
Hi @hlopko! I want to make sure I'm not missing anything first - it's been awhile since I contributed here. If someone can confirm that what I'm saying makes sense, I think I'd prefer to open a separate PR to try and fix the regression test for #9202, and keep this PR centered on #5999.
Great catch, @shaldengeki - that test does look like it is out of place and isn't getting run as it should be.
Thanks for confirming @JasonLunn!
I'll convert this to a draft -- looks like the FFI tests are failing and I have to figure out how to run those locally.
@hlopko Mind running the tests on this? When I run the FFI tests locally I see that the basic tests pass, but I get a segfault in the GC tests when importing generated code, and I want to rule out a local issue.
Hmmm, interesting. So, this is failing in Github:
FAIL: //ruby/compatibility_tests/v3.0.0/tests:basic (see /workspace/_build/out/execroot/com_google_protobuf/bazel-out/k8-fastbuild/testlogs/ruby/compatibility_tests/v3.0.0/tests/basic/test.log)
INFO: From Testing //ruby/compatibility_tests/v3.0.0/tests:basic:
==================== Test output for //ruby/compatibility_tests/v3.0.0/tests:basic:
/workspace/_build/out/external/protobuf_bundle/lib/jruby/3.1.0/gems/power_assert-2.0.3/lib/power_assert.rb:8: warning: tracing (e.g. set_trace_func) will not capture all events without --debug flag
/workspace/_build/out/external/protobuf_bundle/lib/jruby/3.1.0/gems/power_assert-2.0.3/lib/power_assert.rb:8: warning: tracing (e.g. set_trace_func) will not capture all events without --debug flag
Error: Your application used more stack memory than the safety cap of 2048K.
Specify -J-Xss####k to increase it (#### = cap size in KB).
Specify -w for full java.lang.StackOverflowError stack trace
================================================================================
But this passes for me locally, even when I copy the Bazel invocation used in that action:
~/protobuf$ bazel test //ruby/compatibility_tests/v3.0.0/tests:basic //ruby/tests:ruby_version --test_env=KOKORO_RUBY_VERSION --test_env=BAZEL=true --//ruby:ffi=enabled --test_env=PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION=FFI --keep_going --test_output=errors --test_timeout=600
INFO: Found 2 test targets...
INFO: Elapsed time: 296.151s, Critical Path: 62.97s
INFO: 1130 processes: 487 internal, 643 linux-sandbox.
INFO: Build completed successfully, 1130 total actions
//ruby/compatibility_tests/v3.0.0/tests:basic PASSED in 0.7s
//ruby/tests:ruby_version PASSED in 0.2s
Executed 2 out of 2 tests: 2 tests pass.
There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are.
I.... wonder if I need to bump the test size?
@JasonLunn WDYT?
@shaldengeki - the tests that are stack overflowing are doing so under JRuby.
If you've bazel to build the tests on your local under MRI previously, you likely need run bazel clean --expunge, then use RVM to select to your JRuby version, then re-run your bazel test invocation.
Thanks @JasonLunn (and @hlopko)! Can't believe I missed that this was a JRuby thing.
I'm having difficulty reproducing the error. First, I bazel clean --expunged, then I installed JRuby using rvm and selected it:
charles@AlterCation:~/protobuf$ ruby --version
jruby 9.4.6.0-SNAPSHOT (3.1.4) 2023-12-16 627d8d1667 OpenJDK 64-Bit Server VM 11.0.18+10-LTS on 11.0.18+10-LTS +jit [x86_64-linux]
Then I reran my test invocation, which still passed:
bazel test //ruby/compatibility_tests/v3.0.0/tests:basic //ruby/tests:ruby_version --test_env=KOKORO_RUBY_VERSION --test_env=BAZEL=true --//ruby:ffi=enabled --test_env=PROTOCOL_BUFFERS_RUBY_IMPLEMENTATION=FFI --keep_going --test_output=errors --test_timeout=600
# ...
INFO: Found 2 test targets...
INFO: Elapsed time: 198.274s, Critical Path: 50.68s
INFO: 1180 processes: 502 internal, 671 linux-sandbox, 7 worker.
INFO: Build completed successfully, 1180 total actions
//ruby/compatibility_tests/v3.0.0/tests:basic PASSED in 6.8s
//ruby/tests:ruby_version PASSED in 4.9s
Executed 2 out of 2 tests: 2 tests pass.
There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are.
For what it's worth, I'm on an older merge-base, to work around https://github.com/protocolbuffers/protobuf/issues/15030. I can try switching to another platform, but if you've got a moment and want to try reproducing this error on my branch, I'd be much obliged.
Aha, no need -- I'm able to reproduce the StackOverflowError on a different machine; will look into it, and put this in draft status while I figure it out.
I saw that we removed the 3.0.0 compatibility tests from main (which this PR was failing on), so I pulled main into this. Mind if we rerun the tests to see if they pass now?
We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment.
This PR is labeled inactive because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.
@shaldengeki - would you mind merging main into this branch one more time?
All of the vanilla Ruby tests are now passing, but the JRuby tests now have a conformance failure and the FFI variants have a stack overflow.
Are you able to run either of those tests configurations locally?
I can reproduce both errors locally! Appreciate it; let me take a look at this and get back to you.
We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active, please add a comment.
This PR is labeled inactive because the last activity was over 90 days ago. This PR will be closed and archived after 14 additional days without activity.
We triage inactive PRs and issues in order to make it easier to find active work. If this PR should remain active or becomes active again, please reopen it.
This PR was closed and archived because there has been no new activity in the 14 days since the inactive label was added.