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

Decide on a solution for regression testing #179

Closed
qstrahl opened this issue Mar 26, 2014 · 8 comments
Closed

Decide on a solution for regression testing #179

qstrahl opened this issue Mar 26, 2014 · 8 comments

Comments

@qstrahl
Copy link
Collaborator

qstrahl commented Mar 26, 2014

As per the discussion in #178, I'd like to open a dialogue about maintaining regression tests for this project. @gabrielelana fronted some great ideas, and it seems having some test coverage would be both doable and very helpful.

I think it's safe to say that an ideal solution would be node-based, npm-installable, and well-maintained. Some cursory research turned up jasmine-node, which looks promising. I'll look into the viability of putting it to use for this project, and I encourage other interested devs to do the same, or to add their thoughts to the discussion here.

@dragn
Copy link
Contributor

dragn commented Feb 6, 2015

I think that the code for this plugin need a serious revision/rewrite, but before that we really need tests. After a quick search I found some plugins that use Ruby gem 'vimrunner' for testing. But I agree that our tests should better be node-based. Does anyone know of any vimrunner-like npm modules? The closest thing I found: https://github.com/pose/node-remote-vim, but it is, apparently, misses some needed features, like command execution, and the README says that is only supported on OS X. Anyway, I don't think that it will be very difficult to implement it from scratch, or based on pose's implementation.

@dragn dragn mentioned this issue Mar 6, 2015
@markrian
Copy link

It'd be great if this project got some tests in place. It seems like there's not been much or any activity on this in a while, though. Would you accept a PR, perhaps based on what's presented in #254?

@bounceme
Copy link
Collaborator

when this issue was created indentation was in a terrible state. if you believe it is in need of some sort of javascript based testing, how could this improve our development? I personally have a file which i use for testing the bugs and new additions, while i make changes all i need to do is gg=G along with occasionally testing after :%s/^\s*//. most edge cases not handled by the current indenter are concessions for code readability

@markrian
Copy link

markrian commented May 31, 2016

What motivated me to comment here is the fact that I've been bouncing around various commits of vim-javascript trying to find one which indented all of my code correctly. v1.0.0 almost got it right for me, but didn't handle multiline comments correctly. The other commits I tried each seemed to fix one thing but break something else.

I don't care whether or not the testing is JavaScript based; I'm simply in favour of any automated tests. At the very least, why not add this file that you personally use for testing to the repository, so that contributors can use it to verify that their fixes don't cause any regressions?

@bounceme
Copy link
Collaborator

#435 (comment)
also, i'll test https://raw.githubusercontent.com/Khan/tota11y/master/build/tota11y.js for performance, I agree that it is not really ideal. Having tests is great, however if all they add is more code when all they do can be achieved with a small mapping isn't that beneficial. obviously this isn't my repo, though I have recently been doing a lot to the indentation part, make a pr if you want to

@amadeus
Copy link
Collaborator

amadeus commented Jun 21, 2016

Gonna close #254 because it's related to this one, but leaving a ref here since it had some good ideas.

@markrian
Copy link

I've been meaning to get a PR together for this. It'd be similar to what was suggested in #254, except it'd just use a simple bash script. This means no extra dependencies on node, etc. It'd even be possible to have it run in TravisCI.

@amadeus
Copy link
Collaborator

amadeus commented Sep 16, 2016

Closing this due to lack of activity. We can always re-open if the discussion gets rolling again.

@amadeus amadeus closed this as completed Sep 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants