jorgenschaefer / emacs-buttercup

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

Need option to not print skipped tests #161

Closed doublep closed 3 years ago

doublep commented 4 years ago

I'm the author of Eldev build tool and want to add integration with Buttercup library. Currently, Eldev supports only ERT as far as testing goes. Ideally, I want to make Buttercup testing interface as close to what I have for ERT as possible, so that testing framework affects how you write the tests, but not so much how you run them.

However, I stumble at the very first step. In ERT there are selectors. They provide a more-or-less simple way to pick which tests you want run. For that matter, testing libraries I use for other languages (e.g. JUnit in Java) provide similar facilities. But I don't see anything comparable in Buttercup. Is the only way to "select" tests in Buttercup to avoid loading certain files or disable unwanted tests with xit?

jorgenschaefer commented 4 years ago

Hm. Buttercup is mainly meant to be used non-interactively, so you might stumble on some issues.

There is buttercup-run-at-point, and buttercup-run-discover supports a pattern argument, but there is no direct API function you can use.

doublep commented 4 years ago

meant to be used non-interactively

There might be some misunderstanding here. Eldev is also a non-interactive tool, it is run from command-line (of course you can use e.g. Eshell for this, but in any case it is disconnected from your "normal" Emacs). For example, with ERT you can do:

$ eldev test foo

and it will run all the tests with foo in the name and give you results — all in the terminal. One of the purposes, of course, is to run on a continuos integration server, where you actually want to run all the tests anyway. But it is also useful during normal development in cases where you don't plan to interactively debug test failures. For example, because it shields you from all other miscellaneous packages in your normal Emacs, incomplete and obsolete evaluations and so on.

buttercup-run-discover

This looks promising, thank you. In particular, the patterns look somewhat like ERT selectors. In Eldev I'd need to parallel this function inside the tool, but this itself should be no problem.

Two questions. In Buttercup project itself this function fails with a misterious error (maybe there are supposed to be additional setup steps, but I couldn't figure out from README.md):

~/git/buttercup$ emacs -batch -f package-initialize -f buttercup-run-discover
Cannot open load file: No such file or directory, ./tests/test-buttercup.el

And is there any compelling reason function buttercup--specs is package-private, i.e. not called buttercup-specs instead?

Thank you.

jorgenschaefer commented 4 years ago

You need to add the correct directory to the load-path to make it work, e.g. using -L . or similar. You can see the call buttercup uses on Travis CI in the Makefile :-)

doublep commented 4 years ago

Aha, thank you. Maybe you should amend README.md accordingly, because currently it mentions a shell command that just doesn't work.

Now if I run e.g. like this:

$ emacs -batch -L . -f package-initialize -f buttercup-run-discover -p unary

it does run only certain test or two with "unary" in the description. But unfortunately it drowns me in a ton of lines with "SKIPPED" text. This makes it very difficult to say if it passed or failed, and if failed then how exactly. I even have troubles saying how many tests it actually ran: it gave me 220 lines of output. What if there were 10 times as many tests and 2000 lines of output?

So, is it possible to make Buttercup to just not say anything about skipped tests and only print what is important — results of tests I asked for using patterns?

snogge commented 4 years ago

The current default reporter has no way of doing that. It could either be extended with an option - I've sometimes wished for one myself - or you could write a special reporter for your use-case. I've written a reporter that outputs junit, it's on MELPA as buttercup-junit.

I'm guessing you are not interested in maintaining a separate reporter for this purpose, but it could be a very nice solution. It all depends on how you plan to read the data.

doublep commented 4 years ago

The current default reporter has no way of doing that. It could either be extended with an option

OK, would Buttercup accept a patch for that? I think it would be a useful feature irrespective of using Eldev or not.

I'm guessing you are not interested in maintaining a separate reporter for this purpose

No, not really. I'm rather interested in integrating it with Eldev and also having as little integration code in Eldev as possible. I'd prefer to put required improvements into Buttercup itself where they are not really Eldev-specific.

snogge commented 4 years ago

The current default reporter has no way of doing that. It could either be extended with an option

OK, would Buttercup accept a patch for that? I think it would be a useful feature irrespective of using Eldev or not.

What will Eldev do with the output data?

I would prefer that we think about a machine-readable format that is separate from buttercups current output. I'm guessing we could probably emulate ERT output or TAP or something else. But if all Eldev does is pass the output on to the user there is not much point in that, if the package developer has chosen buttercup as their testing framework he/she probably prefers the normal buttercup output.

buttercup does support the use of several simultaneous reporters, so you could get the normal output on stdout and a machine readable report in a file.

I'm guessing you are not interested in maintaining a separate reporter for this purpose

No, not really. I'm rather interested in integrating it with Eldev and also having as little integration code in Eldev as possible. I'd prefer to put required improvements into Buttercup itself where they are not really Eldev-specific.

I'm really glad that you want to integrate buttercup in Eldev, so I'm sure if we can help with that integration in any way we will.

doublep commented 4 years ago

What will Eldev do with the output data?

Normally (as in: for ERT, the only framework currently supported) it just lets the testing library write to stderr/stdout on its own. Basically, let the user see the familiar output, more or less. For ERT it does a few tweaks, but that is controllable by the user (i.e. whether he wants them or not).

I would prefer that we think about a machine-readable format that is separate from buttercups current output.

It sounds like a good goal, but not for this issue. It's of no use to Eldev unless you can standardize this across at least a few different frameworks, which sounds unlikely, seeing e.g. how different Buttercup is compared to ERT.

But if all Eldev does is pass the output on to the user there is not much point in that, if the package developer has chosen buttercup as their testing framework he/she probably prefers the normal buttercup output.

Users are always free to run Buttercup directly, bypassing Eldev, if they find it suits them better (or even not using Eldev at all). Point of having Eldev+Buttercup (or Eldev+ERT, whatever) is to let Eldev handle package dependencies, building (most packages don't need that, but not all) etc. I.e. workflow would look like this:

A real example where Eldev is useful (of course it is my own project again, but...). Tests of library datetime are basically "compare results of this Elisp implementation with those of a known working implementation in Java". So, before running tests I need to build the Java program. With Eldev I can do that automatically: clone repository and (provided you have Eldev and Java compiler) you can instantly run eldev test. The project uses ERT, but it would be exactly the same if it used Buttercup — once Eldev can run it.

buttercup does support the use of several simultaneous reporters, so you could get the normal output on stdout and a machine readable report in a file.

Eldev also has concept of "test runners" and I'm sure it will get extended to invoke appropriate things in Buttercup. Anyway, as I said no-one forces anyone to use Eldev. There are advantages of using Eldev as a wrapper over lots of things, including a test framework. If someone doesn't need them, doesn't like them or doesn't find them compelling enough, he can use Buttercup directly, Cask or whatever else in any combination.

snogge commented 4 years ago

What will Eldev do with the output data?

Normally (as in: for ERT, the only framework currently supported) it just lets the testing library write to stderr/stdout on its own. Basically, let the user see the familiar output, more or less. For ERT it does a few tweaks, but that is controllable by the user (i.e. whether he wants them or not).

So you want a "do not show skipped tests" option to just make this output shorter / more manageable?

Users are always free to run Buttercup directly, bypassing Eldev, if they find it suits them better (or even not using Eldev at all).

Of course. My point was that if Eldev does nothing with the test output for ERT, why should it do something with the buttercup output? Isn't buttercups normal output what the user expects?

If you as a user of Eldev + buttercup want less output from buttercup when using selectors, that's not really about the Eldev-buttercup integration but a buttercup usability issue.

I'm trying to understand if there are technical requirements that need to be met for this integration or if this is more of a personal preferrence.

Both are valid points - as I said I've also sometimes wanted to filter out skipped tests - but the solutions could be vastly different.

On a separate point I see that Eldev supports filtering both with filenames and selectors. Buttercup does not have filename filtering, so that is something to consider.

doublep commented 4 years ago

So you want a "do not show skipped tests" option to just make this output shorter / more manageable?

Yes. But just to make it clear: it's not about command-line option, I'd like to have a defvar in buttercup that makes existing reporters omit all output for skipped tests.

My point was that if Eldev does nothing with the test output for ERT, why should it do something with the buttercup output? Isn't buttercups normal output what the user expects?

Ah, now I understand you. Of course it's not strictly needed, but I want to achieve some uniformity between the supported frameworks. At least because Eldev aims to provide uniform interface to running tests, of course with necessary tweaks and support for additional features of certain frameworks. In ERT when you filter something out using selectors, it doesn't write anything about it. And ideally I'd like something like that also for Buttercup.

It's also a bit of personal preference: I simply don't understand how patterns in the current state are useful at all, since the important output is basically drowned in useless (from my point of view) information. This is similar to one of the reasons often given for fixing warnings, not only errors. You can evaluate each warning and find that this particular one is harmless. But if you ignore them, in a while you'll have a hundred of warnings and no way to spot one that's really important.

On a separate point I see that Eldev supports filtering both with filenames and selectors. Buttercup does not have filename filtering, so that is something to consider.

This should work automatically. Eldev wouldn't call function buttercup-run-discover, but instead do something similar to that on its own, which includes loading test files — optionally only those matching filter. That's why, by the way, I asked if there was any reason for buttercup--specs function to be package-private. Of course I'd also prefer if pattern handling was broken out into its own function (then also the question about buttercup--specs would be obsolete), so that I could just call it instead of repeating these eight lines of code in Eldev integration code.

snogge commented 4 years ago

Yes. But just to make it clear: it's not about command-line option, I'd like to have a defvar in buttercup that makes existing reporters omit all output for skipped tests.

I think a defvar to control this would be fine. It would be nice if it came with an option for the bin/buttercup script, but that can always be added later.

Eldev wouldn't call function buttercup-run-discover, but instead do something similar to that on its own, which includes loading test files — optionally only those matching filter. That's why, by the way, I asked if there was any reason for buttercup--specs function to be package-private.

I wasn't clear on whether you planned to use buttercup directly or starting a separate Emacs instance. From what you say here it sounds like you plan to call it directly. So what we really need is a good API for loading, selecting, and running suites and specs.

Of course I'd also prefer if pattern handling was broken out into its own function (then also the question about buttercup--specs would be obsolete), so that I could just call it instead of repeating these eight lines of code in Eldev integration code.

I have done something along those lines in #158. It's still an internal function though. It's not merged yet because I wanted to write some better tests. I chose to only implement the current selection mechanism, but it should be fairly easy to extend it with more customizable selectors.

doublep commented 4 years ago

I think a defvar to control this would be fine.

I will create a PR for this if you don't mind. By the way, what I also had in mind is something along the lines of ert-quiet (Emacs 27): don't print anything unless it's a failure. Eldev sets this variable when it is started with -q option (not the default). I guess implementation would align nicely with that of omitting skipped tests.

So what we really need is a good API for loading

For me this is not important, Eldev loads files on its own. It handles everything about project structure itself.

selecting, and running suites and specs.

This is definitely the job of the framework. But please preserve backward compatibility if you introduce new API.

I have done something along those lines in #158

Will comment in that PR directly.

snogge commented 3 years ago

@doublep , could you take a look at #184. One of the things I'd like input on is the way I've specified which tests should be suppressed. In #162 and #166 you write in bin/buttercup that

--silent-skipping, -s   When a test is skipped due to patterns or 
                        `xit' form, don't print anything about it.
                        Likewise, don't omit all suite output if all
                        specs inside it are skipped.

but the actual test checks whether the status is pending and the description is "SKIPPED", which only is true for tests that have been skipped using the --pattern option or the buttercup-mark-skipped function.

With this solution, you can use the pending status to suppress all pending tests, whether they are declared with xit, skipped with buttercup-mark-skipped or skipped using assume or buttercup-skip.

I guess which variants of pending to suppress is a matter of personal taste or circumstances, but what do you think is the most useful to bind the options --silent-skipping and --quiet to?

Also, as a consumer of the elisp API, do you think you need further ways to suppress test output? The obvious extension would be to allow functions or failure regexps in buttercup-reporter-batch-quiet-statuses.

doublep commented 3 years ago

With this solution, you can use the pending status to suppress all pending tests, whether they are declared with xit, skipped with buttercup-mark-skipped or skipped using assume or buttercup-skip.

I guess which variants of pending to suppress is a matter of personal taste or circumstances, but what do you think is the most useful to bind the options --silent-skipping and --quiet to?

I'd say --silent-skipping should omit all output resulting from explicit filtering, but show tests that "skipped themselves" for whatever reason. So, tests skipped using buttercup-mark-skipped and probably xit (I'm not a Buttecup user myself, hard to say) would be omitted, everything else shown. Or maybe it can be reformulated like this: if a test is not even started, don't print anything; if a test is started, but cancelled midway (e.g. via buttercup-skip), show this.

--quiet should just print failures and nothing else.

Also, as a consumer of the elisp API, do you think you need further ways to suppress test output? The obvious extension would be to allow functions or failure regexps in buttercup-reporter-batch-quiet-statuses.

No, not currently.

doublep commented 3 years ago

Crap, what I forgot about is that I need the number of skipped tests. As far as I can see, it is not possible to retrieve when using buttercup-mark-skipped, or am I missing something?

doublep commented 3 years ago

~/eldev/test/project-e$ eldev test has Running 2 specs.

Project D tests has a dummy passing test (4.58ms)

Ran 1 out of 2 specs, 0 failed, in 39.69ms.

Looks like Buttercup itself could benefit of such thing. Here, before running the tests it prints the total number, rather than the number of tests it is going to run.

snogge commented 3 years ago

./bin/buttercup -L . tests --no-skip print Running 167 out of 178 specs. before the individual specs are listed. Would you mind filing an issue for this with some information about how you set up buttercup in this case?

doublep commented 3 years ago

Yeah, looks rather like a bug in Eldev integration code:

~/eldev/test/project-e$ ~/git/buttercup/bin/buttercup -L ~/git/buttercup/ --no-skip -L . . -L .eldev/28.0/packages/dependency-a-1.0/ -p has Running 1 out of 2 specs.

Project D tests has a dummy passing test (52.98ms)

Ran 1 out of 2 specs, 0 failed, in 53.53ms.

snogge commented 3 years ago

Just realized that I did not use patterns in my example above, which might make a difference. But you did use patterns, so maybe I was right anyway?

doublep commented 3 years ago

Yes, it was a bug in Eldev, ignore it. I also now use the same way as Buttercup when printing "Running..." to find the number of specs, which solves my original question two hours ago.

This one is really done.