junegunn / vader.vim

A simple Vimscript test framework
589 stars 40 forks source link

Don't strip the parentheses from "Include" #67

Closed mhinz closed 5 years ago

mhinz commented 8 years ago

Hi,

The syntax allows using Include (Syntax): feature/syntax.vader, which is nice, but the parentheses simply get skipped in s:read_vader().

It would be nice if the content of the parens actually got written to the output, so one could avoid prefixing all the tests: https://github.com/mhinz/vim-startify/blob/master/test/feature/syntax.vader#L5-L22

junegunn commented 8 years ago

Hi, thanks for the suggestion. But I should mention that it may not be a good idea at the moment to use Include macro for loading vader files with actual test cases, because the quickfix entry for an error from the included file points to the Include: line instead of the failed test case. It's pretty dumb, unfortunately. You can consider passing test files as the arguments to :Vader command, for example, :Vader setup.vader features/* cleanup.vader, instead of having a container file with includes.

Having said that, I think it's a valid suggestion. One thing to note is that inclusion can be done in multiple depths, so we have to decide how to deal with the message stack.

By the way, you might want to consider using AssertEqual in your test cases. It will give you better error message when it fails.

mhinz commented 8 years ago

Oh. Hmm. Oh! The output of :Vader one two totally works for me as both files are clearly separated in the output. I looked at https://github.com/junegunn/vader.vim/blob/master/test/vader.vader and blindly copied the approach, I guess. :-)

This also eliminates another pain point I had:

Include: file1
Include: file2

If you have Before/After/Given in file1, they will also get executed in file2, which (makes sense but) can be very confusing.

One thing to note is that inclusion can be done in multiple depths, so we have to decide how to deal with the message stack.

Hm, not sure what's the best way to visualise nested calls. Maybe the whole approach is overkill in the first place and we should simply allow using Log in file scope (opposed to the current "block" scope), so folks can simply put a message at the top of the test files.

junegunn commented 8 years ago

If you have Before/After/Given in file1, they will also get executed in file2, which (makes sense but) can be very confusing.

Yeah, the behavior makes sense in some contexts, but it doesn't in the others.

# Sets up Before / After / Given
Include: setup.vader

# Test  cases
Do:
  ...
Execute:
   ...

# Clears Before / After / Given
Include: cleanup.vader

At any rate Include can be confusing if you don't know the exact workings of it.

we should simply allow using Log in file scope

That's a very interesting approach.

mhinz commented 5 years ago

Vader is good enough as it is. Closing. :)