Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Summary #183

Open
wants to merge 26 commits into
base: master
Choose a base branch
from
Open

Summary #183

wants to merge 26 commits into from

Conversation

blueyed
Copy link
Collaborator

@blueyed blueyed commented Nov 21, 2018

Includes/based on #107.

TODO:

  • make this optional?! - most useful only with large test suites with a lot of output (e.g. Neomake)
  • adjust README (example output)
  • add tests?!

@codecov-io
Copy link

codecov-io commented Nov 21, 2018

Codecov Report

Merging #183 into master will decrease coverage by 4.16%.
The diff coverage is 79.2%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #183      +/-   ##
==========================================
- Coverage   86.32%   82.16%   -4.17%     
==========================================
  Files          12       13       +1     
  Lines         775      824      +49     
==========================================
+ Hits          669      677       +8     
- Misses        106      147      +41
Flag Coverage Δ
#nvim ?
#vim 82.16% <79.2%> (-2.75%) ⬇️
Impacted Files Coverage Δ
autoload/vader/window.vim 64.28% <100%> (-5.06%) ⬇️
test/feature/error-location-include.vim 50% <50%> (ø)
autoload/vader.vim 81.48% <76.04%> (-7.54%) ⬇️
autoload/vader/parser.vim 89.9% <92.85%> (-3.43%) ⬇️
ftplugin/vader.vim 88.88% <0%> (-3.71%) ⬇️
autoload/vader/assert.vim 94.44% <0%> (-1.86%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec9b562...3cda9d9. Read the comment docs.

This wraps the temporary script into a function to get the line number
of `vader#assert` calls.
E.g.

    ( 1/13) [EXECUTE] (X) Vim(finish):E168: :finish used outside of a sourced file
      > /home/user/src/neomake/tests/utils.vader:11: finish  " foo

Also passes it through to `s:execute`.
This will use the actual file name for included lines.

Fixes junegunn#88.
Wrapping the code in a function will cause them to not be available otherwise,
or g:SyntaxAt() would have to be used.
While this means more overhead in general, it allows to access
script-local variables in the tests.

squash! FEAT: move s:prepare into vader#window#execute

Use `vader#log` directly instead of `:Log`, since the latter might not
be defined (yet).
This not only avoids multiple calls to `tempname()`, but makes Vim
use the same script ID also when not run in Docker.
For some reason it uses a new script ID (`<SID>`/`<SNR>`) when run
normally, but uses the same ID when run in Docker.
This means that the `!` with `function! s:vader_wrapper()` was not
required in an environment that behaves like running it in Docker (with
Neomake's tests).

This also removes the `delete()`, which is handled by Vim itself when
shutting down.
Vim does not generate profiling information for dict functions when the
dict is local-scoped, since it apparently gets garbage collected
automatically.

Works around vim/vim#2350.
This is relevant for when `Include` is used.

This is a breaking change, although the doc is not clear about it:

> The path of the current `.vader` file can be accessed via `g:vader_file`.
`v:throwpoint` is empty in the first case.
This gets handled by Vim automatically, and works around making Docker
use the same inode or something similar
(vim/vim#3353).
This is supported with Vim patch v8.1.0362.
TODO:

- [ ] make this optional?
- [ ] adjust README
- [ ] add tests?!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants