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

Fix misleading exception in has_entry #513

Conversation

cstyles
Copy link
Contributor

@cstyles cstyles commented Jun 12, 2021

If has_entry is called without any arguments, it will raise an
exception like the following:

ArgumentError: Too many arguments; use either a single argument (must be a Hash) or two arguments (a key and a value).

This is because we don't check if the options.length is zero so when
it is, the else branch executes and prints the above error. We can fix
this by simply adding another branch to cover the case when
options.length is zero.


Unfortunately, this change introduces a new RuboCop violation:

$ bundle exec rake test
Running RuboCop...
Inspecting 234 files
................................................................................................................................................................................
............................................C.............

Offenses:

lib/mocha/parameter_matchers/has_entry.rb:43:5: C: Metrics/CyclomaticComplexity: Cyclomatic complexity for has_entry is too high. [7/6]
    def has_entry(*options) # rubocop:disable Naming/PredicateName ...
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

234 files inspected, 1 offense detected
RuboCop failed!

We could disable the cop but I didn't feel like it was my call to make. I'll leave that up to y'all.

If `has_entry` is called without any arguments, it will raise an
exception like the following:

```
ArgumentError: Too many arguments; use either a single argument (must be a Hash) or two arguments (a key and a value).
```

This is because we don't check if the `options.length` is zero so when
it is, the `else` branch executes and prints the above error. We can fix
this by simply adding another branch to cover the case when
`options.length` is zero.
@floehopper
Copy link
Member

@cstyles Many thanks for this. It looks good to me. I'm having some issue with the Travis CI builds at the moment and I'm a bit short of time, so I might not get it merged for a few days. Please bear with me.

floehopper pushed a commit that referenced this pull request Jun 18, 2021
PR: #513

Previously if has_entry was called without any arguments, it raised the
following exception:

    ArgumentError: Too many arguments; use either a single argument
                   (must be a Hash) or two arguments (a key and a value).
@floehopper
Copy link
Member

I've fixed this up a bit and merged it in 9c4ef13. Thanks again for this contribution. 😄

@floehopper floehopper closed this Jun 18, 2021
@floehopper
Copy link
Member

floehopper commented Jun 25, 2021

Released in v1.13.0.

@cstyles cstyles deleted the fix-misleading-error-message-in-has-entry branch June 25, 2021 16:40
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