freerange
Show proof of concept for keyword arg matching
Attempt a proof of concept for addressing https://github.com/freerange/mocha/issues/446. The unit tests all pass on ruby 3.1.2p20 (2022-04-12 revision 4491bb740a) [arm64-darwin21].
This applies ruby2_keywords (as recommended in the Ruby announcement) to Mock#method_missing and Expectation#with so that the last positional hash will be marked if it is a keyword arg in >Ruby 2.7. This is then checked in a matcher ("LastPositionalHash") if the #with expectation explicitly requests for keyword args.
I think this may break certain test suites, if they were expecting a keyword arg but actually passing in a last positional hash, so it might be good to make this a config first before enforcing the change.
I have not tested this against actual code yet.
TODO:
- [x] Is there a missing unit test scenario in ExpectationTest?
- [ ] Add acceptance tests, c.f. ParameterMatcherTest and/or OptionalParameterMatcherTest.
- [ ] Work out whether the two test failures in StubbaExampleTest is a sign that this change might break a lot of existing tests. Do we need to put this change behind a configuration option?
- [x] Simplify/centralize Ruby version checking for ruby2 keywords functionality. Fixed by using RUBY_V3_PLUS.
- [x] Work out what to do with Ruby v1.8 - there is a problem parsing expectation_test.rb with keyword arguments. Resolved in https://github.com/freerange/mocha/pull/536
- [x] Resolve Rubocop issues in unit tests. Resolved by disabling Style/BracesAroundHashParameters cop and using hash rocket syntax in combination with Hash.ruby2_keywords_hash to designate keyword arguments. Now that we've dropped support for Ruby v1.8, we could probably change the configuration of the Style/HashSyntax cop, but that feels like a bigger change (see https://github.com/freerange/mocha/issues/537).
- [x] Fix Yardoc for LastPositionalHash. Fixed by marking the whole class as private from a documentation point of view.
- [ ] Find someone with a large Rails codebase using Mocha to test a pre-release version
@floehopper, happy to help! Would I commit directly to that PR? Our branch off and we merge to that PR?
@floehopper, happy to help! Would I commit directly to that PR? Our branch off and we merge to that PR?
Great - thank you! I don't think you'll have permission to add commits to that PR, so it's probably simplest to add to this PR and I can copy changes across. It might not be too hard to get Circle CI builds working on your forked repo if that would help - let me know if I can help with that.
Would I commit directly to that PR? Our branch off and we merge to that PR?
Actually, I've done some more work in the branch for #535, so it might be worth branching off that in your fork and opening a new PR. Alternatively you could amend the branch for this PR.
Ok, I've setup circleci and cherry-picked your commits onto this branch, will find some time this week to continue!
Ok, I've setup circleci and cherry-picked your commits onto this branch, will find some time this week to continue!
Brilliant - thank you!