Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reproduction case for Ruby 3.2 and kwargs detection #1497

Closed
wants to merge 4 commits into from

Conversation

pirj
Copy link
Member

@pirj pirj commented Nov 5, 2022

Related to #1495

@pirj pirj self-assigned this Nov 5, 2022
@pirj
Copy link
Member Author

pirj commented Nov 5, 2022

@eregon Do you happen to know what might have caused the failures on 3.2.0-preview2 and ruby-head while passed just fine with 3.1?

Is this still correct for Ruby 3.2?

        if RSpec::Support::RubyFeatures.distincts_kw_args_from_positional_hash? && expected_args == actual_args
          expected_hash = expectation.expected_args.last
          actual_hash = args_for_multiple_calls.last.last
          if Hash === expected_hash && Hash === actual_hash &&
            (Hash.ruby2_keywords_hash?(expected_hash) != Hash.ruby2_keywords_hash?(actual_hash))
            actual_args += Hash.ruby2_keywords_hash?(actual_hash) ? " (keyword arguments)" : " (options hash)"
            expected_args += Hash.ruby2_keywords_hash?(expected_hash) ? " (keyword arguments)" : " (options hash)"
          end
        end

2.2 failure is unrelated, to be fixed by #1498

Comment on lines +135 to +136
expect(dbl).to receive(:kw_args_method).with({a: 1, b: 2})
dbl.kw_args_method(a: 1, b: 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this expected to fail? It expects a positional Hash but is given kwargs.
But I guess expect(dbl).to receive actually creates a method with no kwargs so then it should pass?

Copy link
Member Author

Choose a reason for hiding this comment

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

This indeed should have failed on earlier Ruby versions, too.

@eregon
Copy link
Contributor

eregon commented Nov 5, 2022

That code looks fine. I suspect it's a missing ruby2_keywords on the chain of calls, and so the flag is lost somewhere.
You can debug by printing Hash.ruby2_keywords_hash?(h) before the exception happens.

@eregon
Copy link
Contributor

eregon commented Nov 5, 2022

And 3.2 fixes a bug where some missing ruby2_keywords accidentally kept the flag: https://github.com/ruby/ruby/blob/master/NEWS.md

@pirj
Copy link
Member Author

pirj commented Nov 5, 2022

Thank you so much for the explanation, @eregon !

benoittgt added a commit that referenced this pull request Jan 3, 2023
pirj pushed a commit that referenced this pull request Jan 4, 2023
Related:
  - #1497
  - #1502

Co-authored-by: Slava Kardakov <ojab@ojab.ru>
@pirj
Copy link
Member Author

pirj commented Jan 4, 2023

Superseded by #1514

@pirj pirj closed this Jan 4, 2023
@pirj pirj deleted the ruby-3.2-weird-kwargs-detection branch January 4, 2023 14:40
pirj pushed a commit that referenced this pull request Jan 4, 2023
Related:
  - #1497
  - #1502

Co-authored-by: Slava Kardakov <ojab@ojab.ru>
pirj pushed a commit that referenced this pull request Jan 4, 2023
Related:
  - #1497
  - #1502

Co-authored-by: Slava Kardakov <ojab@ojab.ru>
pirj pushed a commit that referenced this pull request Jan 4, 2023
Related:
  - #1497
  - #1502

Co-authored-by: Slava Kardakov <ojab@ojab.ru>
JonRowe pushed a commit that referenced this pull request Jan 7, 2023
Related:
  - #1497
  - #1502

Co-authored-by: Slava Kardakov <ojab@ojab.ru>
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.

None yet

2 participants