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

Add unit tests for the contribution system #110

Closed
syl20bnr opened this issue Nov 12, 2014 · 8 comments
Closed

Add unit tests for the contribution system #110

syl20bnr opened this issue Nov 12, 2014 · 8 comments
Assignees

Comments

@syl20bnr
Copy link
Owner

Much needed :-)

@lazywithclass
Copy link
Contributor

This is what I got so far, it's pretty basic, it's testing spacemacs/git-has-remote:

(load-file "../init.el")

(ert-deftest git-has-remote ()
  (should (equal (spacemacs/git-has-remote "clearly-not-a-R3M0T3!") nil))
  (should (numberp (spacemacs/git-has-remote "origin"))))

I've got some questions, to see if I'm headed in the correct way:

  1. do I really need to load init.el to bring into the context the git related function? Could you recommend a better way?
  2. this test look more of an integration test, rather than a unit test, is that fine?
  3. do you want me to cover all the git related functions before a push or could we do it somewhat incrementally?

Sorry if all of this seems trivial, I have zero experience in testing Emacs Lisp :b

@syl20bnr
Copy link
Owner Author

@lazywithclass We are on the same boat, I've never done unit testing in elisp so I need practice like you.

  1. I don't think we need to load init.el, you can set the variables you need in a let special form.
  2. you are right, I thought about that, the git functions are integration/functional tests. We should clearly separate unit and functional testing, having sub-folders unit and functional or integration would be cool. So we could run them is separate Travis step if needed. It is also important for development.
  3. Ideally yes, all the git functions should be functionally tested to be sure we correctly integrate the 3rd party tool.

I'm for small and incremental PRs.

@lazywithclass
Copy link
Contributor

@syl20bnr I wrote a couple more tests for spacemacs/git-has-remote and spacemacs/git-fetch-tags.
You can have a look at the latest version of the test or at the commit I'm talking about.

I am not submitting a PR because I feel like it's just a first draft and would really like some feedback from you.
I included a library called mocker.el which provides enough syntactic sugar to deal with mocks, I've created a folder for that: core/test/lib.
I am sorry but I didn't get your hint about how not to load init.el.
I still have to separate them into the proper unit and integration folders.

Do you think I'm headed in the right direction?

@syl20bnr
Copy link
Owner Author

@lazywithclass I looked at it and it is a very good beginning.
I improved the test framework and pushed it in develop branch (see commit description 1280f82)

The necessary modifications for your test file are in this gist: https://gist.github.com/syl20bnr/96fd26431dc77aed5996
Put it in core/test and you can submit a PR.

Also if you want to put your email in the file header.

Thank you for bootstrapping the tests, I did not have enough motivation to start them ahah

Note: Travis build is broken until you submit your PR.

@lazywithclass
Copy link
Contributor

@syl20bnr I will move to a new house in this weekend, it will be really intense :D! I will do my best to have it done by tonight since the build is broken.

Thank you for putting my name in the header; it's a pleasure to help.

@syl20bnr
Copy link
Owner Author

@lazywithclass Don't worry about spacemacs. I'll just fix the makefile for now.
Good luck!

@hijarian
Copy link
Contributor

hijarian commented Jul 1, 2015

@syl20bnr Just to be clear: those tests for new contributions, what are their goals?

New functionality in new contributions should be covered by it's own tests, written by contributor (in ideal world).

Do you mean a set of regression tests checking that new contribution doesn't break existing feature set? Then it looks more like automatic runnable Spacemacs specification, a bit larger in scope than "unit tests for contribution system". :)

@syl20bnr
Copy link
Owner Author

syl20bnr commented Aug 9, 2015

I'm fine with the current unit tests although the coverage is pretty low, the foundation is enough tested to be able to reliably reason about the higher level algorithms.

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

No branches or pull requests

4 participants