2018-02-19 0:30 GMT+03:00 Bram Moolenaar <[email protected]>:
>
> As mentioned in the discussion about the popup menu fix, it's not so
> easy to write a test to check that something shows up in the right
> place, with the right highlighting, etc.  Basically, that Vim displayes
> the right text.  And when the check fails, it's not so easy to figure
> out what changed.
>
> If have just submitted patch 8.0.1523, which adds functionality to work
> with terminal screen dumps.  The idea is to write a test that puts Vim
> in a certain state, create a screen dump with term_dumpwrite() and check
> that it is equal to a reference screen dump. I added assert_equalfile()
> to make it easier to check if two files are equal.
>
> This should also make it a lot easier to check syntax highlighting: Load
> text for the language, enable syntax highlighting and create a screen
> dump.  Any differences in highlighting from a reference dump will be
> easily found.
>
> If there is a difference, you can use term_dumpdiff() to find out where
> it is.  It shows the text that is different, and pressing "s" swaps
> between the two versions, so you can see the context of the change.
> You can use term_dumpload() to view a screen dump.
>
> I haven't yet had time to actually write a test with this, thus things
> might still change.  And it probably breaks in some cases.
>
> I also need to write an explanation of how to use this properly.  It's
> not going to replace the existing tests.  To use it in a portable way
> the size of the dump needs to be limited.
>
> Let me know what you think.

Looks like overly complicated to use. Neovim has screen tests for
quite some time now and it has key properties regarding those tests
that make them convenient to use:

1. Tests use screen of a certain known dimensions, and creating a
screen with those dimensions is a matter of writing two lines: one to
create a screen saving object into a variable and second to attach
Neovim instance to that screen. This convenience is only a matter of
creating an utility library and needs not actually be done in C code
for Vim as well, even though Vim is going to use :terminal and Neovim
is using remote UI written specifically for tests. I do not know what
does max-height and max-width do, but supplying them to
term_dumpwrite() of all things looks like something too late to
matter: screen needs to be formed before calling this function and it
needs to be formed with certain dimensions.
2. Another thing is that Neovim utility library handles waiting for
specific screen state with a timeout, and should catch that screen
state even if it is intermediate and not final one (though this
functionality is normally not needed). I do not know whether it would
be possible to catch intermediate states in Vim and in any case using
files is not going to make waiting fast which goes for the third
problem.
3. Screen state is described completely in test code. Format is easy
to read and write by humans and it does not need documentation, but
even if term_dumpwrite() file format was easy to read (it looks like
it is not actually, described in next point) ***it dumps contents to a
separate file***. Don’t do this for tests, new style tests are better
not because they are in the same Vim instance (only good for
performance and bad for anything else), but because they keep expected
result close to the code which is intended to produce it what is
easier to track and debug. Separate file is a step back.
4. File format is unreadable garbage. Format of the diff buffer is
neither readable unless you have thrice the screen height and terminal
height is small in first place. Compare the format from Vim (also in
test.tdump as I am pretty sure gmail will mangle text):

    
|/+0#d0d0d0255#1c1c1c255|h|o|m|e|/|z|y|x|/|.|l|o|c|a|l|/|b|i|n|/|v|i|r|t|u|a|l|e|n|v|w|r|a|p@1|e|r|.|s|h|
@77
    
|/+0#ff404010&|h|o|m|e|/|z|y|x|/|.|z|s|h|/|f|a|s|t|-|s|y|n|t|a|x|-|h|i|g|h|l|i|g|h|t|i|n|g|/|f|a|s|t|-|s|y|n|t|a|x|-|h|i|g|h|l|i|g|h|t|i|n|g|.|p|l|u|g|i|n|.|z|s|h|:|2|6|5|:|
|s|c|a|l|a|r| |p|a|r|a|m|e|t|e|r|
|Z|S|H|_|H|I|G|H|L|I|G|H|T|_|M|A|X|L|E|N|G
    |T|H| |c|r|e|a|t|e|d| |g|l|o|b|a|l@1|y| |i|n| |f|u|n|c|t|i|o|n|
|(|a|n|o|n|)| +0#d0d0d0255&@80
    | +2#ffffff255#0087af255|z|y|x| |+0#0087af255#585858255|
|~+0#bcbcbc255&|/|O|p|e|r|a|D|o|w|n|l|o|a|d|s|/|f|a|r|t|/|c+2#d0d0d0255&|o|s|p|l|a|y|
|+0#585858255#1c1c1c255| | +0#d0d0d0255&@79
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119
    @119

with Neovim equivalent:

    screen:expect([[
      ^/home/zyx/.local/bin/virtualenvwrapper.sh            |
      {2:/home/zyx/.zsh/fast-syntax-highlighting/fast-syntax-h}|
      {2:ighlighting.plugin.zsh:265: scalar parameter ZSH_HIGH}|
      {2:LIGHT_MAXLENGTH created globally in function (anon)}  |
      {3: zyx }{4: }{5:…/Proj/c/}{6:neovim }{7: }     {8: }{9: 
list-reimplement } |
                                                           |
                                                           |
                                                           |
                                                           |
                                                           |
                                                           |
                                                           |
      {1:term://.//9394:zsh                                   }|
                                                           |
    ]], {[1] = {bold = true, reverse = true}, [2] = {foreground =
Screen.colors.Brown1, special = Screen.colors.Grey0}, [3]= {special =
Screen.colors.Grey0, background = 26265, bold = true, foreground =
Screen.colors.Grey100}, [4] = {background = 5789784, foreground =
26265, special = Screen.colors.Grey0}, [5] = {background = 5789784,
foreground = Screen.colors.Gray78, special = Screen.colors.Grey0}, [6]
= {special = Screen.colors.Grey0, background = 5789784, bold = true,
foreground = 14540253}, [7] = {foreground = 5789784, special =
Screen.colors.Grey0}, [8] = {foreground = 2894892, special =
Screen.colors.Grey0}, [9] = {background = 2894892, foreground =
Screen.colors.Gray78, special = Screen.colors.Grey0}})

Note that while both are autogenerated in *actual* code this may look like

      screen:set_default_attr_ids({
        EOB={bold = true, foreground = Screen.colors.Blue1},
        T={foreground=Screen.colors.Red},
        RBP1={background=Screen.colors.Red},
        RBP2={background=Screen.colors.Yellow},
        RBP3={background=Screen.colors.Green},
        RBP4={background=Screen.colors.Blue},
      })

    …

        screen:expect([[
                                   |
          {EOB:~                        }|
          {EOB:~                        }|
          {EOB:~                        }|
          {RBP1:(}{RBP2:()}{RBP1:)}^                     |
        ]])

: nothing could prevent you from using more human-readable names (EOB
is End-Of-Buffer abbreviation, RBP1 is RainBowParenthesis1
abbreviation: not using abbreviations is possible, but that will bloat
code too much for my taste). Test suite also allows not emitting
`{group:…}` for some specific highlighting (mostly used for Normal)
and caret marks cursor position.

The key points here:

  1. No visual garbage for each character. What is more readable
`|z|y|x` or `zyx`?
  2. Highlighting is kept separately from text.
  3. Highlighting may be given readable names.
  4. Readable names for colors if they match some predefined ones.
  5. Cursor position is easy to find.

(BTW, after seeing this nonsense for such complex case created by
Neovim’s screen:snapshot_util (equivalent to term_dumpwrite) I wrote
two patches so that autogenerated text will look like

    screen:expect([[
      ^/home/zyx/.local/bin/virtualenvwrapper.sh            |
      {2:/home/zyx/.zsh/fast-syntax-highlighting/fast-syntax-h}|
      {2:ighlighting.plugin.zsh:265: scalar parameter ZSH_HIGH}|
      {2:LIGHT_MAXLENGTH created globally in function (anon)}  |
      {3: zyx }{4: }{5:…/Proj/c/}{6:neovim }{7: }     {8: }{9: 
list-reimplement } |
                                                           |
                                                           |
                                                           |
                                                           |
                                                           |
                                                           |
                                                           |
      {1:term://.//27928:zsh                                  }|
                                                           |
    ]], {
      [1] = {bold = true, reverse = true},
      [2] = {foreground = Screen.colors.Brown1, special = Screen.colors.Grey0},
      [3] = {special = Screen.colors.Grey0, background = 0x006699,
bold = true, foreground = Screen.colors.Grey100},
      [4] = {background = 0x585858, foreground = 0x006699, special =
Screen.colors.Grey0},
      [5] = {background = 0x585858, foreground = Screen.colors.Gray78,
special = Screen.colors.Grey0},
      [6] = {special = Screen.colors.Grey0, background = 0x585858,
bold = true, foreground = 0xdddddd},
      [7] = {foreground = 0x585858, special = Screen.colors.Grey0},
      [8] = {foreground = 0x2c2c2c, special = Screen.colors.Grey0},
      [9] = {background = 0x2c2c2c, foreground = Screen.colors.Gray78,
special = Screen.colors.Grey0},
    })

after they will be merged.)

5. I did not see documentation of the dumps format in the latest patch
which means that new functionality *must not be used* outside of Vim
own test suite. Neovim has this functionality available only in its
own test suite now and does not attempt to export this, but if new
functionality is to be used by third-party plugin developers it needs
to be subject to backwards compatibility, which needs creating
documentation.

>
> --
> My girlfriend told me I should be more affectionate.
> So I got TWO girlfriends.
>
>  /// Bram Moolenaar -- [email protected] -- http://www.Moolenaar.net   \\\
> ///        sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
> \\\  an exciting new programming language -- http://www.Zimbu.org        ///
>  \\\            help me help AIDS victims -- http://ICCF-Holland.org    ///
>
> --
> --
> You received this message from the "vim_dev" maillist.
> Do not top-post! Type your reply below the text you are replying to.
> For more information, visit http://www.vim.org/maillist.php
>
> ---
> You received this message because you are subscribed to the Google Groups 
> "vim_dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to [email protected].
> For more options, visit https://groups.google.com/d/optout.

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Attachment: test.tdump
Description: Binary data

Raspunde prin e-mail lui