jorgenschaefer / emacs-buttercup

Behavior-Driven Emacs Lisp Testing
GNU General Public License v3.0
360 stars 44 forks source link

Likely unintended behavior change in commit 41424d5 #191

Closed doublep closed 3 years ago

doublep commented 3 years ago

Patterns are no longer treated as regexps because of these lines in 41424d5:

-      (buttercup--mark-skipped buttercup-suites patterns))
+      (buttercup-mark-skipped (regexp-opt patterns) t))

Function regexp-opt accepts strings not regexps.

In comparison, previously each pattern was treated as regexp:

-      (unless (cl-dolist (p patterns)
-                (when (string-match p spec-full-name)
snogge commented 3 years ago

Command line patterns are documented to be substrings, not regexps in the shell script help. That is probably not prominent enough. Was this issue on the command line or through the api?

doublep commented 3 years ago

It's not really an issue, I just remember that it used to be regexp and noticed this suspicious change. If this is intentional (i.e. if it was behaving incorrectly before), it is not a problem, I will adjust Eldev accordingly then, so that Eldev/Buttercup behaves the same as Buttercup.

doublep commented 3 years ago

Basically, I need a clarification what the intended behavior is, because I cannot pass a list of strings to buttercup-mark-skipped for it to decide how to treat them.

snogge commented 3 years ago

So the change was intentional but probably a bad idea. I chose to do this change to keep the code simpler, but didn't really gain much.
Please check if #192 works for you.

doublep commented 3 years ago

Yes, seems to work fine:

~/git/buttercup$ eldev test "raised?" Running 14 out of 162 specs.

The buttercup-failed signal can be raised (2.14ms)

The buttercup-pending signal can be raised (0.21ms)

The `buttercup-expect' function with a function as a matcher argument should not raise an error if the function returns true (0.26ms) should raise an error if the function returns false (0.21ms) [...]

Ran 14 out of 162 specs, 0 failed, in 95.27ms.

snogge commented 3 years ago

Would you be interested in writing a section in the docs/running-tests.md file about using eldev with buttercup? Maybe you want some stuff finished first?

doublep commented 3 years ago

Basically it would be a repeat of parts of Eldev's documentation, but if you want, I can do it.

Maybe you want some stuff finished first?

Currently missing (to bring Buttecup support in Eldev on par with Eldev) are #190 (trivial), #188 and also maybe #185 (this one should also be simple; if implemented, I will add support for this for both ERT and Buttercup in one go). Also maybe a way to reuse previous test results, but this would not be easy and would need to be designed first.