add test for "Better Method#inspect" introduced in Ruby 2.7

[lxxxvi]

Hello 👋

From #745

• Method#inspect shows more information. Feature #14145

I hope I've chosen the right approach and put it in the right place?!

[andrykonchin]

Looks good 👍 .

But I would add some corner cases with *a, **b, &c etc. They could be based on specs from Ruby own tests https://github.com/ruby/ruby/pull/2618/files#diff-4ed63f6118a737e9326cb7c64b07a527R635-R657.

[eregon]

There are quite few methods in

spec/core/method/fixtures/classes.rb

Lines 49 to 79 in bfd843a

	def zero; end
	def one_req(a); end
	def two_req(a, b); end
	def zero_with_block(&blk); end
	def one_req_with_block(a, &blk); end
	def two_req_with_block(a, b, &blk); end
	def one_opt(a=nil); end
	def one_req_one_opt(a, b=nil); end
	def one_req_two_opt(a, b=nil, c=nil); end
	def two_req_one_opt(a, b, c=nil); end
	def one_opt_with_block(a=nil, &blk); end
	def one_req_one_opt_with_block(a, b=nil, &blk); end
	def one_req_two_opt_with_block(a, b=nil, c=nil, &blk); end
	def two_req_one_opt_with_block(a, b, c=nil, &blk); end
	def zero_with_splat(*a); end
	def one_req_with_splat(a, *b); end
	def two_req_with_splat(a, b, *c); end
	def one_req_one_opt_with_splat(a, b=nil, *c); end
	def two_req_one_opt_with_splat(a, b, c=nil, *d); end
	def one_req_two_opt_with_splat(a, b=nil, c=nil, *d); end
	def zero_with_splat_and_block(*a, &blk); end
	def one_req_with_splat_and_block(a, *b, &blk); end
	def two_req_with_splat_and_block(a, b, *c, &blk); end
	def one_req_one_opt_with_splat_and_block(a, b=nil, *c, &blk); end
	def two_req_one_opt_with_splat_and_block(a, b, c=nil, *d, &blk); end
	def one_req_two_opt_with_splat_and_block(a, b=nil, c=nil, *d, &blk); end

It would be good to test the output for each "kind" of argument (pre/opt/rest/post/keyreq/keyopt/keyrest/block).

[eregon]

Could you use m.send(@method).should.include?("two_req(a, b)")?
That way, there is no need to escape any character.

[lxxxvi]

Hello @andrykonchin and @eregon

Thank you for your feedbacks. 🎉

Extending the tests revealed a new trivial issue: line size.
First I came up with this, but I think it's not very readable.

it "returns a String containing method arguments" do
  obj = MethodSpecs::Methods.new
  obj.method(:zero).send(@method).should.include?("#zero()")
  obj.method(:one_req).send(@method).should.include?("one_req(a)")
  obj.method(:two_req).send(@method).should.include?("two_req(a, b)")
  # ... there are more ...
  obj.method(:one_req_one_opt_with_splat_and_block).send(@method).should.include?("one_req_one_opt_with_splat_and_block(a, b=..., *c, &blk)")
  obj.method(:two_req_one_opt_with_splat_and_block).send(@method).should.include?("two_req_one_opt_with_splat_and_block(a, b, c=..., *d, &blk)")
  obj.method(:one_req_two_opt_with_splat_and_block).send(@method).should.include?("one_req_two_opt_with_splat_and_block(a, b=..., c=..., *d, &blk)")
end

Putting the inputs into a "table" and then loop makes it somewhat more readable, but slower (I guess?).

it "returns a String containing method arguments" do
  obj = MethodSpecs::Methods.new
  [
    [:zero, "#zero()"],
    [:one_req, "one_req(a)"],
    [:two_req, "two_req(a, b)"],
    # ... there are more ...
    [:one_req_one_opt_with_splat_and_block, "one_req_one_opt_with_splat_and_block(a, b=..., *c, &blk)"],
    [:two_req_one_opt_with_splat_and_block, "two_req_one_opt_with_splat_and_block(a, b, c=..., *d, &blk)"],
    [:one_req_two_opt_with_splat_and_block, "one_req_two_opt_with_splat_and_block(a, b=..., c=..., *d, &blk)"]
  ].each do |method_key, expected_string|
    obj.method(method_key).send(@method).should.include?(expected_string)
  end
end

Do you have any preferences here?
Or should I put all assertions into individual tests?

[eregon]

I think we can just omit the method name in the String given to include?:

it "returns a String containing method arguments" do
  obj = MethodSpecs::Methods.new
  obj.method(:zero).send(@method).should.include?("()")
  obj.method(:one_req).send(@method).should.include?("(a)")
  obj.method(:two_req).send(@method).should.include?("(a, b)")
  # ... there are more ...
  obj.method(:one_req_one_opt_with_splat_and_block).send(@method).should.include?("(a, b=..., *c, &blk)")
  obj.method(:two_req_one_opt_with_splat_and_block).send(@method).should.include?("(a, b, c=..., *d, &blk)")
  obj.method(:one_req_two_opt_with_splat_and_block).send(@method).should.include?("(a, b=..., c=..., *d, &blk)")
end

The table gives less information in the case an expectation fails, so it's not as good.

Another possibility would be to just break lines like:

it "returns a String containing method arguments" do
  obj.method(:one_req_one_opt_with_splat_and_block).send(@method)
    .should.include?("one_req_one_opt_with_splat_and_block(a, b=..., *c, &blk)")
end

As mentioned in #793 (comment) I think it's useful to test each kind of argument, but not all combinations, so the 3 *_req_*_opt_with_splat_and_block are redundant and only one of them is useful to test.

[lxxxvi]

Hello @eregon

Once again, thank you for your review and feedback!

I think we can just omit the method name in the String given to include?

Perfect, done. It fits. 🚀

However, I'm bit intimidated by the comment for testing "each kind of argument", as I am insecure what some of them mean 🙃

"pre/opt/rest/post/keyreq/keyopt/keyrest/block"

However, I added tests for each simple "kind" of argument, plus one "complicated" case.
Please let me know if you're missing some cases.

Thanks

[eregon]

This looks great, thank you!