Conversation
Replace Oga with Nokogiri and address frozen string compatibility
- Replace Oga dependency with Nokogiri for HTML parsing
- Add minitest-mock, logger, and ostruct as explicit dependencies
- Fix frozen string mutations by using String.new or .dup where needed
- Ensure UTF-8 encoding for StringIO in logger tests
- Update logger to handle encoding for non-UTF-8 strings when sanitize_encoding is not set
- Update test helper to use Nokogiri API (element['attr'] instead of element.get('attr'))
- Conditionally require minitest-mock gem only for Ruby >= 3.2 - Add version constraint for sqlite3 gem on Ruby < 3.1 - Wrap minitest/mock requires in rescue blocks for older minitest versions - Reorder logger gem declaration for consistency
|
@achiurizo Can you review this pull request? I'm thinking of merging this in and then at some point releasing 0.16.2. Should allow the full Padrino test suite to be green across all Ruby versions from 2.7 to Ruby 4. Since this only affects test files, I'll wait to release another version to rubygems. But this fixes the build matrix |
achiurizo
left a comment
There was a problem hiding this comment.
LGTM — nice work getting Ruby 4.0 compat in. Left a few minor comments.
| else | ||
| line.encode!('UTF-8', invalid: :replace, undef: :replace) unless line.encoding == Encoding::UTF_8 |
There was a problem hiding this comment.
This changes behavior on Ruby 3.x too — binary/non-UTF-8 data that previously wrote through untouched will now get replacement characters when @sanitize_encoding isn't set.
Consider scoping to the log stream's encoding so it only kicks in when the write would actually fail:
| else | |
| line.encode!('UTF-8', invalid: :replace, undef: :replace) unless line.encoding == Encoding::UTF_8 | |
| line.encode!(@log.external_encoding || Encoding::UTF_8, invalid: :replace, undef: :replace) unless line.encoding == Encoding::UTF_8 |
…andling - Remove Ruby version check for minitest-mock gem dependency - Remove ostruct gem from development dependencies - Update logger to use @log.external_encoding when sanitize_encoding is not set - Ensure encoding compatibility with log device's external encoding
- Conditionally require minitest-mock gem only for Ruby >= 3.1 - Ensures compatibility with older Ruby versions where minitest-mock may not be needed
|
Thanks for the review Arthur! Applied all three suggestions, with one caveat: Logger encoding — good catch on the behavior change for 3.x. Now uses @log.external_encoding || Encoding::UTF_8 so it only encodes when the write would actually fail against the stream's encoding. minitest-mock — I tried removing the Ruby version guard, but minitest-mock resolves to 5.27.0, which declares required_ruby_version >= 3.1. Bundler fails at resolution time on Ruby 2.7/3.0 before the begin/rescue LoadError in the test helpers ever gets a chance to run. Kept the guard but adjusted it to if RUBY_VERSION >= '3.1' to match the gem's own requirement. ostruct — removed the redundant Gemfile entry since it's already in the gemspec. Pushed the updates, tests still green across all sub-gems. Going to merge as soon as all the tests pass green. |
Summary
Fixes test suite to pass on Ruby 4.0 (and continues to pass on Ruby 3.x). Ruby 4.0 introduced several breaking changes that affected padrino-framework:
Changes
Frozen string fixes (
padrino-core/lib/padrino-core/application/routing.rb)Symbol#to_snow returns a frozen string in Ruby 4.0. Route paths derived from symbols (e.g.get :show) were being mutated in-place withgsub!,<<, etc., causingFrozenError. Fixed by calling.dupon the result of.to_sand on frozen string paths before mutation.new_url = ''toString.newinrebase_url.Removed default gems
ostruct: Removed from Ruby 4.0 stdlib. Added as a runtime dependency inpadrino-core.gemspecsincepadrino-core/configuration.rbrequires it.logger: Removed from Ruby 4.0 stdlib. Added to Gemfile (needed byrack-protection).Replaced
ogawithnokogiri(padrino/test/padrino/test-methods.rb)ogadepends onruby-ll, which has a C extension that fails to compile on Ruby 4.0. Replaced withnokogiriwhich is actively maintained and Ruby 4.0 compatible. Only used in test assertions.Minitest mock extraction (
padrino-core/test/helper.rb,padrino-cache/test/helper.rb)Object#stubwas extracted from minitest 6.0 into the separateminitest-mockgem. Added the gem andrequire 'minitest/mock'to test helpers that use.stub.Logger encoding fix (
padrino-core/lib/padrino-core/logger.rb,padrino-core/test/test_logger.rb)StringIOdefaults to US-ASCII. Binary data is now encoded to UTF-8 with replacement before writing.Test Results (Ruby 4.0.2)
All 9 sub-gems pass: 1,441 runs, 5,465 assertions, 0 failures, 0 errors.