add specs for CGI.escapeURIComponent

[lxxxvi]

Hi again 👋 (it's been a while)

This PR tries to address the first part of

• CGI.escapeURIComponent and CGI.unescapeURIComponent are added.
  [Feature #18822]

from #1016 .

I mainly took over the tests from the implementation and added a few more.
Let me know what makes sense and what doesn't.

I can add specs for .unescapeURIComponent in another PR, if that's OK?

[andrykonchin]

suggestion: we could add some other cases:

• for implicit type conversion of an argument (to String with method #to_str)
• for ascii-compatible String and ascii-not-compatible (it treats a String as a bytes sequence and converts bytes, not characters)

[lxxxvi]

I'm a bit uncertain here. I added a spec for the implicit type conversion, but I'm not sure if I understood it correctly what you suggested. Please let me know if I misunderstood.

I could use a pointer regarding the ascii-not-compatible string idea... how could I approach this? Thanks for your help, I appreciate it!

[andrykonchin]

Regarding implicit conversion - you are correct. Actually you may look for similar test cases, e.g. here:

spec/core/basicobject/instance_eval_spec.rb

Lines 287 to 301 in db5f026

	it "converts string argument with #to_str method" do
	source_code = Object.new
	def source_code.to_str() "1" end
	a = BasicObject.new
	a.instance_eval(source_code).should == 1
	end
	it "raises ArgumentError if returned value is not String" do
	source_code = Object.new
	def source_code.to_str() :symbol end
	a = BasicObject.new
	-> { a.instance_eval(source_code) }.should raise_error(TypeError, /can't convert Object to String/)
	end

[andrykonchin]

for ascii-compatible String and ascii-not-compatible (it treats a String as a bytes sequence and converts bytes, not characters)

I could use a pointer regarding the ascii-not-compatible string idea... how could I approach this?

I meant that most of the test cases use ASCII characters. But the method actually handles multibyte encodings, e.g. UTF-8/UTF-16/UTF-32 and just converts their bytes one by one (ignoring "characters" that could contain several bytes):

CGI.escapeURIComponent("β")
# => "%CE%B2"

"β".bytes.map {|c| c.to_s(16) }
# => ["ce", "b2"]

[andrykonchin]

suggestion: usually it makes sense to add exception message when it's stable among supported Ruby versions.

[lxxxvi]

Here I'm a bit uncertain too... I now added an additional assertion for the error message, is this what you meant? If not, let me know so I can fix it.

[andrykonchin]

Yeah, it's what I meant. But it could be simplified a bit as far as raise_error accepts String or Regexp to match an exception message as a second argument, e.g.:

-> { }.should raise_error(ArgumentError, /duplicate/)
-> { }.should raise_error(TypeError, "non-hash or symbol given")

[lxxxvi]

Thanks for that hint. I managed to miss that one in CONTRIBUTING.md, apologies. Will fix it.

[andrykonchin]

Looks good!

I can add specs for .unescapeURIComponent in another PR, if that's OK?

I believe it's completely OK.

[lxxxvi]

@andrykonchin Thanks a lot for your precious feedback. I pushed some commits which should address at least some of your feedbacks.

[andrykonchin]

Could you please also squash commits?

Will do it myself.

[andrykonchin]

Thank you!