kpumuk commented on PR #3270: URL: https://github.com/apache/thrift/pull/3270#issuecomment-3678115067
After a careful consideration, I decided to rewrite the tests merged in https://github.com/apache/thrift/pull/2223, and move them from `lib/rb` to `test/rb`. Here is why: * These are more of a protocol integration tests, and act as a pre-check for the cross-tests * They depend on test-unit, which is the framework used in `test/rb` Original architecture depended on `fork` call, which does not cooperate with test-unit. For example, compact protocol test was failing in the client on non-accelerated builds (Ruby version returned `""` as the first element, while binary versions returns `nil`). The tests never picked that because exit code was coming from the client, which only writes and has no assertions, so never fails. Additionally, artificial delay of 2 seconds was injected into every test to coordinate between server and client. Instead, the new approach uses threads and a queue to signal to the client when server is ready. It correctly asserts in the main threads and so the exit code is always correct. ``` ============================================================================================================================================ Failure: test_different_data_types(TestCompactProtocol) core/protocol/test_compact_protocol.rb:177:in 'TestCompactProtocol#test_different_data_types' 174: assert_equal("[6, 8, 12]", "#{server_results[:acc_map1]}") 175: assert_equal("[0, 0, 0]", "#{server_results[:acc_map2]}") 176: assert_equal("[8, 5]", "#{server_results[:acc_set]}") => 177: assert_equal("[nil, 6, 5]", "#{server_results[:acc_field1]}") 178: assert_equal("[nil, 0, 0]", "#{server_results[:acc_field2]}") 179: end 180: end <"[nil, 6, 5]"> expected but was <"[\"\", 6, 5]"> diff: ? [nil, 6, 5] ? "" ? ??? ============================================================================================================================================ Finished in 0.01072975 seconds. -------------------------------------------------------------------------------------------------------------------------------------------- 1 tests, 16 assertions, 1 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications 0% passed ``` The change in the `Bytes` was made to address a warning in Ruby 3.4+: ``` /code/lib/rb/lib/thrift/bytes.rb:35: warning: literal string will be frozen in the future (run with --debug-frozen-string-literal for more information) ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
