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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syntax Specs #178

Closed
wants to merge 1 commit into from
Closed

Syntax Specs #178

wants to merge 1 commit into from

Conversation

gabrielelana
Copy link

This is a rudimental way to test that a change in the syntax file don't cause some unwanted regression.

This is based on the Golden Master approach:

  • When you run the tests for the first time, it generates an html file for each file in spec/fixtures directory, those files are the html export (:help :TOhtml) of the current file highlighted with the syntax found in syntax/javascript.vim
  • Those html files are the Golden Master aka the known state of the current syntax that is supposed to be good
  • Next time you run the tests the produced html file will be compared to the Golden Master of the same file, if the diff is not empty then the test fails
  • When you change something in the syntax you could test the effects of this change running the tests and inspecting the diffs
  • When you think that the current status is good then you need to regenerate the Golden Master

Right now I have included only one file in the fixtures (the latest version of jquery) as an example, ideally we should have small files with specific examples, more the example is focused and more the diff will be helpful. The best way is to build those fixtures incrementally feature by feature

INSTALLATION AND USAGE: Install Ruby 2.0 and Bundler, run bundle install in the root directory, run rspec to run all the tests, run GENERATE_GOLDEN_MASTER=1 rspec to regenerate the Golden Master files

DOCUMENTATION: If you might like this I would add proper development documentation (in CONTRIBUTING.md?)

ONE LAST THING: in a local .vimrc file I've added some mappings that I found very useful

  • <Leader>r (short for reload) will reload the plugin in the current buffer for rapid feedback when you change something
  • <Leader>h (short for help) will show the syntax attributes of the element under the cursor
  • <Leader>t (short for trace) will enter in a sub mode that shows the syntax attributes of the element under the cursor when you move (warning: could be pretty slow but it's really helpful sometimes)

I have used the same approach Here and it paid off pretty soon 馃槃

This is a rudimental way to test that a change in the file don't cause
some unwanted regression.

This is based on the *Golden Master* approach:
- When you run the tests for the first time, it generates an html file
  for each file in spec/fixtures directory, those files are the html
  export (:help :TOhtml) of the current file highlighted with the syntax
  found in syntax/javascript.vim
- Those html files are the *Golden Master* aka the known state of the
  current syntax that is supposed to be good
- Next time you run the tests the produced html file will be compared to
  the *Golden Master* of the same file, if the diff is not empty then
  the test fails
- When you change something in the syntax you could test the effects of
  this change running the tests and inspecting the diffs
- When you think that the current status is good then you need to
  regenerate the *Golden Master*

Right now I have included only one file in the fixtures (the latest
version of jquery) as an example, ideally we should have small files
with specific examples, more the example is focused and more the
diff will be helpful. The best way is to build those fixtures
incrementally feature by feature

INSTALLATION AND USAGE: Install Ruby 2.0 and Bundler, run `bundle
install` in the root directory, run `rspec` to run all the tests, run
`GENERATE_GOLDEN_MASTER=1 rspec` to regenerate the *Golden Master* files

DOCUMENTATION: If you might like this I would add proper development
documentation (in CONTRIBUTING.md?)

ONE LAST THING: in a local .vimrc file I've added some mappings that I
found very useful
- `<Leader>r` (short for reload) will reload the plugin in the current
  buffer for rapid feedback when you change something
- `<Leader>h` (short for help) will show the syntax attributes of the
  element under the cursor
- `<Leader>t` (short for trace) will enter in a sub mode that shows the
  syntax attributes of the element under the cursor when you move
  (warning: could be pretty slow but it's really helpful sometimes)
@gabrielelana gabrielelana changed the title Syntax pecs Syntax Specs Mar 24, 2014
@davidchambers
Copy link
Collaborator

While I haven't tried this change, I'm very much in favour of it in theory. :)

@qstrahl
Copy link
Collaborator

qstrahl commented Mar 26, 2014

While I'm very much in favour of testing, and this method is sound, I don't favour the execution.

This plugin is typically included by cloning this git repository into ~/.vim/bundle. This change would introduce roughly twenty thousand (!!!) lines of code which will never be needed or used by anyone but us developers (and even then only those among us who are conscientious =P). In addition to this, I have a personal objection to the inclusion of a local .vimrc, which I see as a minuscule convenience for those who like its effects at the expense of great frustration for those who do not; nay nay, I say. Let the developer decide if they want to use those helpers.

I can see two possible solutions to the first problem. Both of them involve maintaining a separate GitHub repository called vim-javascript-tests, or what you will:

  1. Include vim-javascript-tests as a git submodule in this repository so developers can pull it in at will. By default, this code will not be pulled in by git, thus sparing at least those of our users who are using default git settings from 20,000 lines of extraneous code.
  2. Maintain vim-javascript-tests as an independent repository containing helpful testing tools that our developers can clone and use at will, thus sparing all of our users from 20,000 lines of extraneous code.

In my opinion, the above solutions are roughly equally favourable. Both of them address my first (and largest) concern, and I would suggest our devs informally vote to decide which is more preferable if we go that route.

Neither of those solutions address my second concern (that is, the inclusion of additional vim config locally in this project or related projects). Of the offending code, I suggest wholesale castration - not because it is not useful, but because it is in the wrong place. Perhaps we could put it into a gist and add an appropriate helpful reference thereto in CONTRIBUTING.md so our developers know where to get it if they want.

In all cases, CONTRIBUTING.md should be updated to keep everyone informed about our development process.

@qstrahl
Copy link
Collaborator

qstrahl commented Mar 26, 2014

Also worthy of consideration is that not all javascript developers are fluent in ruby, and it may be wise to avoid this proposed exclusion by adopting a javascript-based testing framework instead.

@davidchambers
Copy link
Collaborator

Your reasoning seems sound to me, @qstrahl. My slight preference would be to include the test repository as a submodule, though a standalone repository would be fine.

Also worthy of note is that not all javascript developers are fluent in ruby, and it may be pertinent to avoid this proposed exclusion by adopting a javascript-based testing framework.

If there is an npm-installable equivalent I'd prefer that, but if Ruby dependencies are the price we must pay for regression tests, that's okay with me.

@qstrahl
Copy link
Collaborator

qstrahl commented Mar 26, 2014

If there is an npm-installable equivalent [of rspec] I'd prefer that...

Jasmine is quite insanely popular right now, and also happens to be my personal favourite choice. The typical stack is Grunt for task automation, Karma as your test runner, and Jasmine as your test framework. It's quite easy to manage with npm, and you don't have to use so many components (it's just much nicer). Managing these through npm would also alleviate the concern of bloating the code for our users (to some degree).

@qstrahl
Copy link
Collaborator

qstrahl commented Mar 26, 2014

Mind you that's the stack for testing javascript applications... testing our little vim plugin here may require a different approach, but I'm confident the javascript tools available are up to the task.

@gabrielelana
Copy link
Author

@qstrahl I鈥檓 sorry, I didn鈥檛 explain very well what is the purpose of this pull request: I only wanted to show you guys the approach in a way that would let you play with it, nothing more, nothing less, it鈥檚 not supposed to be merged as is.

As you said, jquery it鈥檚 huge and using it as a fixture it鈥檚 not going to be very helpful, also the local .vimrc it鈥檚 pretty personal and maybe unwelcome by some of you but maybe helpful as inspiration

In short, I wanted to show you guys something, let you play with it, and discuss with you what take and what to leave

@qstrahl
Copy link
Collaborator

qstrahl commented Mar 26, 2014

I鈥檓 sorry, I didn鈥檛 explain very well what is the purpose of this pull request: I only wanted to show you guys the approach in a way that would let you play with it, nothing more, nothing less, it鈥檚 not supposed to be merged as is.

Ah, okay. In the future, you might want to just use a gist for that.

I wanted to show you guys something, let you play with it, and discuss with you what take and what to leave

In that case, mission accomplished. I will close this pull request and open an issue for further discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants