google / vroom

Launch vim tests
Apache License 2.0
275 stars 27 forks source link

Support @skipif directive #116

Open dbarnett opened 4 years ago

dbarnett commented 4 years ago

Sometimes a section of vroom testing only makes sense to run in certain conditions (particular versions of vim etc.). It would be useful to have a @skipif CONDITION directive that could evaluate a vimscript expression and conditionally skip some vroom lines.

Example:

@skipif !has('job')
:call job_start(...)
:echomsg xyz
~ SOME EXPECTED OUTPUT
@endskipif

Then this could note in the test summary output that it passed but with some skipped portions.

dbarnett commented 4 years ago

Note making vroom tests too conditional could quickly get out of hand, with long vroom files being so linear and stateful. Might decide a feature like this would be too hard to get right and not worth building.

An alternative that's already supported is to keep all the conditional stuff in separate files and have the test runner scripts conditionally --skip files they don't support. maktaba does that with vroom/system-vimjob.vroom, for example: https://github.com/google/vim-maktaba/blob/f2abdd19a/.travis.yml#L23.

xanderman commented 4 years ago

Counter proposal: @skipif is only valid as the first directive in the file.

It would be really handy to be able to point vroom --crawl at a directory and not have to figure out ahead of time which tests are going to be valid. Putting it into a directive also gives the option of having vroom give a report of skipped tests, which would be useful for determining if test coverage is sufficient (or could let you know that your vim version isn't what you expected).

Making it only valid as the first directive encourages test decomposition, and also greatly simplifies the interpreter as it can just stop immediately. It would also be simple to implement the directive as "skip the rest of the file". I would be very opposed to vroom guessing where to resume tests (if @skipif were per-test) because beginning/end of a test is kind of ambiguous. It would need a corresponding @resume or something, and that just gets messy for writing tests.

dbarnett commented 4 years ago

Ooh, yeah, I'd thought about doing it as an early exit mechanism but not about making it just a file-level directive.