sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
950 stars 405 forks source link

test: add tests for calc, dice plugins #2503

Closed SnoopJ closed 11 months ago

SnoopJ commented 11 months ago

Description

This changeset adds tests for the calc, dice plugins, inspired by discussion on #2464.

Checklist

dgw commented 11 months ago

Is this intended to replace the tests generated by passing expected output to the @plugin.example decorator?

https://github.com/sopel-irc/sopel/blob/7b74964e2de3f2d0964d9f5b0f56b5e9447bb988/sopel/modules/calc.py#L14-L21

https://github.com/sopel-irc/sopel/blob/7b74964e2de3f2d0964d9f5b0f56b5e9447bb988/sopel/modules/dice.py#L170-L182

dice has helper functions that could be covered by unit tests, but I'm not entirely sure of the value-add here when the whole point of @example is to allow testing the expected output of commands without needing a separate test file. 🤔

SnoopJ commented 11 months ago

Is this intended to replace the tests generated by passing expected output to the @plugin.example decorator?

I don't think I knew that the decorator tested the observed behavior this way! In that case, this can just be closed, it adds nothing they aren't already covering.

dgw commented 11 months ago

it adds nothing they aren't already covering.

It'd be cool to fill in the gaps, though. calc is at 65% and dice is at 77% coverage according to my local report.

Stuff like empty-argument branches, exception handlers, and other error conditions (including within the aforementioned helper functions) are left out, if you're looking for some test improvements you can make to these plugins. 🙂

(Up to you whether that means adding more examples and making judicious use of the user_help kwarg to @example, or covering the edge/error cases in separate test files. Or replacing the example tests with comprehensive coverage in separate test files… I was just pointing out that the minimal, "normal path" tests you had when opening this PR were redundant!)

SnoopJ commented 11 months ago

Up to you whether that means adding more examples and making judicious use of the user_help kwarg to @example, or covering the edge/error cases in separate test files. Or replacing the example tests with comprehensive coverage in separate test files

I suspect more examples can get the coverage up to somewhere we're happier with. Definitely don't want to replace the examples, they're useful documentation, and splitting between examples and tests seems kind of annoying. I'll have a look-see.