Skip to content

Conversation

@alexanderadam
Copy link
Contributor

@alexanderadam alexanderadam commented Oct 28, 2025

This PR just extends the test for UTF-16 LE/BE as well.

refs #52 and #1129 (comment)

PS: I'm looking for a new adventure in case anybody is looking to hire or work with a Ruby/Rails/Crystal dev
PPS: would you be so kind and add the hacktoberfest-accepted label to this issue in case you find that PR helpful? 🥺

@alexanderadam alexanderadam force-pushed the add_additional_encoding_tests branch from 53acae1 to 078782c Compare October 28, 2025 17:49
Comment on lines 352 to 356
if RUBY_ENGINE != 'truffleruby'
test_obj.define_singleton_method("test_utf16_method".encode(Encoding::UTF_16)) {}
end
test_obj.define_singleton_method("test_utf16le_method".encode(Encoding::UTF_16LE)) {}
test_obj.define_singleton_method("test_utf16be_method".encode(Encoding::UTF_16BE)) {}
Copy link
Member

Choose a reason for hiding this comment

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

Only testing one of UTF_16LE or UTF_16BE is enough.

As mentioned in #1129 (comment), UTF_16 is a dummy encoding. Testing UTF_16 is not so important. There's a benefit removing UTF_16 test: removing truffleruby condition.

Here's a list of encodings that cause error. It is not restricted to UTF_16 family.

UTF-16BE	UTF-16LE	UTF-32BE	UTF-32LE	UTF-16	UTF-32	CP949	Emacs-Mule
EUC-TW	Windows-1258	GB1988	macCentEuro	macThai	IBM037	stateless-ISO-2022-JISO-2022-JP
CP50220	CP50221	MacJapanese	ISO-2022-JP-KDDI	stateless-ISO-2022-JP-KDDI

Found by this script

Encoding.list.each do |encoding|
  obj = Object.new
  (128..0xffff).each do |c|
    obj.define_singleton_method('a'.encode(encoding) + c.chr(encoding)){}
  rescue EncodingError, RangeError
  end
  IRB::RegexpCompletor.new.completion_candidates('', 'obj.a', '', bind: binding) rescue puts encoding.name
end

I don't think it's worth testing all of these encodings.
So I think selecting one of them (UTF_16LE or UTF_16BE) is enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it.
Thank you again for the explanation. I updated the test to only check for UTF-16LE now

@alexanderadam alexanderadam force-pushed the add_additional_encoding_tests branch from 078782c to 7af1008 Compare November 3, 2025 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants