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

calc: improve help output, test coverage (now 100%) #2530

Merged
merged 3 commits into from
Nov 1, 2023
Merged

Conversation

dgw
Copy link
Member

@dgw dgw commented Oct 27, 2023

Error cases weren't covered by the @plugin.example tests. Fixed that.

Also declared a few more examples that should appear in the help output, to showcase more of the plugin's capabilities.

Follow-up to #2503 (comment).

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make lint and make test)
  • I have tested the functionality of the things this change touches

@dgw dgw added the Tests label Oct 27, 2023
@dgw dgw added this to the 8.0.1 milestone Oct 27, 2023
@dgw dgw requested a review from a team October 27, 2023 05:10
Copy link
Contributor

@SnoopJ SnoopJ left a comment

Choose a reason for hiding this comment

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

ezpz, +1

@dgw
Copy link
Member Author

dgw commented Oct 28, 2023

@Exirel @SnoopJ I had an itch to scratch in sopel/tools/calculation.py too. 😁

Edit: 09:57:57 <+xrl> dgw: it's still good for me

@dgw dgw modified the milestones: 8.0.1, 8.0.0 Oct 28, 2023
sopel/modules/calc.py Outdated Show resolved Hide resolved
Oops! All Error Cases!

...were not covered by the test suite. That was the easiest 100% ever.
Sopel displays only one example command if none of the examples is
defined as `user_help=True`, and I believe these four happen to show off
the most variety of usage in a suitably succinct way.
@dgw
Copy link
Member Author

dgw commented Oct 31, 2023

With apologies for the force-push, I had to rebase in order to make the wording change addressing @SnoopJ's stylistic concern over one error message. (Blame the merging of #2504.) Scout's honor that that was the only thing I changed, but this diff is small enough that you guys can definitely re-review it if you want between now and when it gets merged.

These additional cases (and a new error handler) cover as much of the
`tools.calculation` submodule as possible using the example decorator.
Further coverage of `tools.calculation` would require real unit tests
of the sort one finds in this project's `test/` directory.
Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

Still LGTM!

@dgw dgw merged commit 0371d0b into master Nov 1, 2023
15 checks passed
@dgw dgw deleted the more-calc-tests branch November 1, 2023 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants