Add Travis integration #61

cg505 · 2017-07-16T16:08:34Z

This PR adds basic integration for Travis CI. Merging it now will allow future PRs to write tests for their code.

cg505 · 2017-07-16T16:09:29Z

This is a part of #53. (Does not fix.)

cg505 · 2017-07-16T16:47:01Z

Gonna squash those last two commits.

rye

This is a good first step. I just that some cleanup specifically related to the Travis config and some of the runners is needed to make this very refined.

rye · 2017-07-16T17:22:56Z

.emacs.d/test/lisp/behavior/completion-c-test.el

+    :var (orig-mode)
+
+    (before-all
+      ; record orignal setting for later


typo: orignal

rye · 2017-07-16T17:29:03Z

.travis.yml

+install:
+  - emacs --script .emacs.d/test/lisp/install.el
+script:
+  - emacs --version


This shouldn't be included in the script block; it's not part of our tests.

Do you advocate removing completely or moving to before_script?

That would be smart, I think.

rye · 2017-07-16T17:29:07Z

.emacs.d/test/lisp/dot-tests.el

+
+(defun kotct/run-tests ()
+  (setf buttercup-suites nil)
+  (load "~/.emacs.d/test/lisp/behavior/completion-c-test.el" nil t)


Is nil t necessary here? Can you extract the test running behavior out into a different function and call it a bunch of times in a master function rather than add more and more of these lines to this one function? Or maybe do globbing to find all the tests?

nil t is necessary, otherwise we get a bunch of messages saying it loaded all the test files. It's annoying. I'm making this line dynamic so we don't have to specify all the test files.

Do you prefer nil t over something more like nil 'nomessage?

That is fine.

rye · 2017-07-16T17:29:10Z

.emacs.d/test/lisp/dot-tests.el

@@ -0,0 +1,27 @@
+(require 'buttercup)
+
+(defun kotct/load-corresponding ()


Missing docstring?

rye · 2017-07-16T17:29:35Z

.travis.yml

+  - export PATH="/home/travis/.evm/bin:$PATH"
+  - git clone https://github.com/rejeep/evm.git /home/travis/.evm
+  - evm config path /tmp
+  - evm install $EVM_EMACS --use --skip


This should move to the install block.

rye · 2017-07-16T17:30:08Z

.travis.yml

+  - EVM_EMACS=emacs-25.2-travis
+before_install:
+  - export PATH="/home/travis/.evm/bin:$PATH"
+  - git clone https://github.com/rejeep/evm.git /home/travis/.evm


Maybe consider cloning before exporting PATH? It's just a matter of semantics.

rye · 2017-07-16T17:31:36Z

.travis.yml

+  - git clone https://github.com/rejeep/evm.git /home/travis/.evm
+  - evm config path /tmp
+  - evm install $EVM_EMACS --use --skip
+  - ln -s "$(pwd)/.emacs.d" ~/.emacs.d


Perhaps do this in the install block as well?

rye · 2017-07-16T17:31:40Z

.travis.yml

+before_install:
+  - export PATH="/home/travis/.evm/bin:$PATH"
+  - git clone https://github.com/rejeep/evm.git /home/travis/.evm
+  - evm config path /tmp


Doesn't matter but maybe use mktemp -d to make a temporary directory specifically for the install?

Don't think this really matters. EVM says to use /tmp, and I don't want to run into issues with that, so I'm just going to leave it.

concerns addressed, please re-review

rye

Progress, but I'm hesitant to keep dev-mode in init.el.

rye · 2017-07-17T01:13:38Z

.travis.yml

+before_script:
+  - emacs --version
+script:
+  - emacs --script ~/.emacs.d/init.el


Is this needed and will it stay persistent to the next call?

This just tests startup. Nothing persists to the next call (except maybe the creation of some random things like the recentf file).

rye · 2017-07-17T01:14:03Z

.travis.yml

+  - emacs --version
+script:
+  - emacs --script ~/.emacs.d/init.el
+  - emacs --script .emacs.d/test/lisp/run-tests.el


Which one of these is needed? Also, do you need ~/ at the start of this?

The first one is intended to make sure that Emacs can successfully start without error. The second runs the tests.

The ~/ is just meant to replicate Emacs' behavior by running init.el from the home directory instead of the clone directory.

Probably we can make the presence of ~/ consistent without impact, I was just explaining why it is what it is rn.

rye · 2017-07-17T01:17:27Z

.travis.yml

+  - EVM_EMACS=emacs-25.1-travis
+  - EVM_EMACS=emacs-25.2-travis
+before_install:
+  - git clone https://github.com/rejeep/evm.git /home/travis/.evm


Consider using $HOME instead; HOME=/home/travis by default.

You could also just use ~, which is equivalent to $HOME.

rye · 2017-07-17T01:18:06Z

.travis.yml

+  - EVM_EMACS=emacs-25.2-travis
+before_install:
+  - git clone https://github.com/rejeep/evm.git /home/travis/.evm
+  - export PATH="/home/travis/.evm/bin:$PATH"


Ditto here, use $HOME or ~?

rye · 2017-07-17T01:18:55Z

.travis.yml

+cache: apt
+env:
+  - EVM_EMACS=emacs-25.1-travis
+  - EVM_EMACS=emacs-25.2-travis


Can we add support for latest HEAD Emacs, too? Does EVM have development versions?

EVM has an emacs-git-snapshot-travis build but it seems like maybe not the best plan to have our tests dependent on unstable behavior in the emacs git repo? It also seems unnecessary. On second thought I don't really care very much and this is unlikely to break anything. I'm happy to go with whatever you think is the best plan.

I typically test my Ruby projects against ruby's latest release(s) as well as the latest Git snapshot because I like to see if stuff works with the new API. It's okay to omit it, though.

rye · 2017-07-17T01:28:51Z

.emacs.d/init.el

@@ -109,6 +116,10 @@ Despite this, your config appears to have loaded successfully.")
  (if loaddefs-buffer
      (kill-buffer loaddefs-buffer)))

+;; If in dev mode, load the test runner.
+(when kotct/dev-mode
+  (require 'dot-tests "~/.emacs.d/test/lisp/dot-tests"))


I'm hesitant to include this (both the var and the thing) in init.el because

It adds calls to the init stream under normal non-dev circumstances, and

Adding this to the source tree for the entire configuration means that we're tainting our actual configuration with testing stuff.

I think tests should be run in a separate Emacs instance from the existing one, if the developer is running one. The ability to run kotct/run-tests within the normal running circumstances is somewhat sketchy to me. Far better to run Emacs using a --script that also loads init than do this, I think.

It adds calls to the init stream under normal non-dev circumstances

Hardly. I wouldn't consider a variable definition and an if-statement to be adding anything.

Adding this to the source tree for the entire configuration means that we're tainting our actual configuration with testing stuff.

I mean... the tests do not run or actually do anything unless you explicitly run them, so the only "tainting" is the definition of a few functions. Also this only happens in dev mode.

I think tests should be run in a separate Emacs instance from the existing one, if the developer is running one. The ability to run kotct/run-tests within the normal running circumstances is somewhat sketchy to me.

Officially, a separate Emacs instance is the correct way to run the tests. If emacs --script ~/.emacs.d/test/lisp/run-tests.el fails, the tests do not pass. That is how they're run in Travis. That said, having the ability to run the tests in your Emacs session is a convenience that makes it much easier to run/debug tests, and there's very little overhead.

This will be a good enough first step, I guess. We can go ahead and continue with the PR, and if it causes problems we can rethink.

rye

LGTM for now! 🎉 🥇

first stab at testing with buttercup

1a0afdf

cg505 requested review from rye and samontea July 16, 2017 16:08

cg505 added the Concerns: Emacs Configuration label Jul 16, 2017

add travis conf

5dfca52

cg505 force-pushed the test-framework branch from 53c1ccb to 5dfca52 Compare July 16, 2017 16:48

rye previously requested changes Jul 16, 2017

View reviewed changes

cg505 added 3 commits July 16, 2017 16:13

dynamically load tests

7dc777c

add dev mode and load tests when in dev mode

23052a1

initialize emacs as part of travis tests

b6af5be

cg505 force-pushed the test-framework branch from 35da326 to b6af5be Compare July 16, 2017 20:13

cg505 requested a review from rye July 17, 2017 00:54

rye requested changes Jul 17, 2017

View reviewed changes

cg505 added 3 commits July 18, 2017 20:27

fix usage of home directory

fd70b32

also test against emacs HEAD

35fdca5

slack notifs + whitespace update

38364fe

cg505 requested a review from rye July 19, 2017 00:58

rye approved these changes Jul 19, 2017

View reviewed changes

rye mentioned this pull request Jul 19, 2017

Testing and CI #53

Open

4 tasks

cg505 merged commit 5d5c678 into master Jul 19, 2017

rye modified the milestone: Version 0.0.0 Aug 3, 2017

cg505 mentioned this pull request Aug 3, 2017

Add Coverage reporting to coveralls.io #75

Merged

rye deleted the test-framework branch September 11, 2017 20:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Travis integration #61

Add Travis integration #61

cg505 commented Jul 16, 2017

cg505 commented Jul 16, 2017 •

edited

Loading

cg505 commented Jul 16, 2017

rye left a comment •

edited

Loading

rye Jul 16, 2017

rye Jul 16, 2017

cg505 Jul 16, 2017

rye Jul 16, 2017

rye Jul 16, 2017

cg505 Jul 16, 2017

rye Jul 16, 2017

cg505 Jul 16, 2017

rye Jul 16, 2017

rye Jul 16, 2017

rye Jul 16, 2017 •

edited

Loading

rye Jul 16, 2017 •

edited

Loading

rye Jul 16, 2017

cg505 Jul 16, 2017

rye left a comment

rye Jul 17, 2017

cg505 Jul 19, 2017

rye Jul 17, 2017

cg505 Jul 19, 2017

cg505 Jul 19, 2017

rye Jul 17, 2017

rye Jul 17, 2017

rye Jul 17, 2017

rye Jul 17, 2017

cg505 Jul 18, 2017

rye Jul 18, 2017

rye Jul 17, 2017

cg505 Jul 19, 2017

rye Jul 19, 2017 •

edited

Loading

rye left a comment

		@@ -0,0 +1,27 @@
		(require 'buttercup)

		(defun kotct/load-corresponding ()

Add Travis integration #61

Add Travis integration #61

Conversation

cg505 commented Jul 16, 2017

cg505 commented Jul 16, 2017 • edited Loading

cg505 commented Jul 16, 2017

rye left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rye Jul 16, 2017 • edited Loading

Choose a reason for hiding this comment

rye Jul 16, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rye left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rye Jul 19, 2017 • edited Loading

Choose a reason for hiding this comment

rye left a comment

Choose a reason for hiding this comment

cg505 commented Jul 16, 2017 •

edited

Loading

rye left a comment •

edited

Loading

rye Jul 16, 2017 •

edited

Loading

rye Jul 16, 2017 •

edited

Loading

rye Jul 19, 2017 •

edited

Loading