Add a travis file, prepare to run tests on travis-ci.org #886

lucc · 2016-07-17T09:03:08Z

From the existence of https://github.com/pazz/alot/tree/0.3.8-unittests I take it that it is also planned to run these test somewhere. So I suggest to use travis and start with this travis file. I already set up travis for my fork of alot so you can see the build for this PR here: https://travis-ci.org/lucc/alot/branches

I initially did some more tests with travis wich can be found in my fork (branches travis-orig and travis-multi-os).

lucc · 2016-07-17T13:21:08Z

I was hoping to do something more thorough in the last commit but I could not think of any way to start alot and quit it immediatly afterwards (like vim -c quit for example). Setting initial_command=exit results in a blank screen. Only after I press q alot quits. This is the corresponding log:

DEBUG:manager:/home/luc/.config/alot/themes
INFO:ui:setup gui in 256 colours
DEBUG:ui:fire first command
DEBUG:ui:global command string: "exit"
DEBUG:__init__:mode:global got commandline "exit"
DEBUG:__init__:ARGS: [u'exit']
DEBUG:__init__:cmd parms {}
ERROR:ui:Could not stop reactor: Can't stop reactor that isn't running..
Shutting down anyway..
DEBUG:ui:Got key (['q'], [113])
DEBUG:ui:cmdline: 'exit'
DEBUG:ui:global command string: "exit"
DEBUG:__init__:mode:global got commandline "exit"
DEBUG:__init__:ARGS: [u'exit']
DEBUG:__init__:cmd parms {}

lucc · 2016-07-18T01:18:33Z

I found a way to exit alot immediately, will clean up the git history next.

pazz · 2016-07-19T20:56:45Z

cool! I have no idea how to use this yet but feel free to integrate the unitttest branch. I'm not working on this atm.

lucc · 2016-07-19T23:26:17Z

In order to use this you have to got to http://travis-ci.org, sign in with you Github account and give travis access to this repository and configure it to run on pull requests. Then you should see the result here next to the merge-conflict-indicator.

If you don't say otherwise I will just cherry-pick your unittests.

pazz · 2016-07-20T08:35:51Z

Quoting Lucas Hoffmann (2016-07-20 00:26:17)

In order to use this you have to got to [1]http://travis-ci.org, sign in
with you Github account and give travis access to this repository and
configure it to run on pull requests. Then you should see the result here
next to the merge-conflict-indicator.

If you don't say otherwise I will just cherry-pick your unittests.

go ahead! thanks. I think i spend some time trying to set up a sensible
notmuch test database to deal with malformed mails. But it wasnt
completely finished..

lucc · 2016-07-20T08:59:09Z

Do you want (me) to add more tests to this PR or are you considering it as is?

ghost

Multiple dependencies (on e.g. the filesystem and notmuch) were introduced by this test - which should be avoided. The reason is that if the Unit test fails, we don't know wether the bug is in notmuch, the filesystem or in our codebase.

As far as i understand unit testing, the solution is to use a mocking framework. Unfortunately, i don't understand mocking enough to implement the changes.

ghost · 2016-12-09T14:26:01Z

test/corpus.py

+        shutil.copytree(path.join(HERE,'corpus'), self.TESTCORPUS)
+
+        # create a temporary notmuch database
+        # create notmuch config file


That is a dependency on the filesystem and on notmuch. That means, this test is not an unit test, rather an functional test (i think). An Unit Test should, according to the book The Art of Unit Testing, have no real dependencies on anything. The reason is that it dependends on the correct working of the filesystem and notmuch - so if the unit test fails, we don't know whether this bug is really in OUR codebase.

pazz · 2016-12-09T14:35:34Z

The thing is that in order to test many features it is necessary to have some dummy data (e.g. email files) to test on.

lucc · 2016-12-09T15:11:47Z

I am not sure if we can get rid of this filesystem dependency as even the test files will be files. Also the alot code base.

Also not sure if it is possible to get rid of the dependency on notmuch. If we want to argue that our code works we have to argue that it uses the notmuch bindings correctly (because that is what happens in production). And I can also try to continue this mocking idea ad absurdum: Why not mock the other python dependencies (urwid, urwidtrees, ...) or even python itself.

Sure this is very artificial and I hope you don't take any offense from it, none was intended. I just have to learn some more about the principles behind unit testing (and have a look at that link).

We could for now also remove the unit testing commits from this PR and focus on getting the travis integration to work. It will then hopefully be easier to add linting/testing/whatnot jobs in the future.

pazz · 2016-12-09T17:08:22Z

Sorry for my ignorance: Why would i want "travis integration" if we already have some automated flake / pep8 comments if not for running unit tests?

lucc · 2016-12-09T18:27:56Z

@pazz you are right, I overlooked that quantifycode gives us linting. The only thing I can think of right now is that travis could run make -C docs html && git diff --exit-code in order to check that the last commiter did update and commit autogenerated documentation.

zenhack · 2016-12-10T05:39:55Z

.travis.yml

+  # gmime is needed to build notmuch from source.
+  - sudo apt-get -yqq install libgmime-2.6-dev
+  # Clone the notmuch repository and move into it.
+  - git clone git://notmuchmail.org/git/notmuch


Observation: this is always going to test against notmuch's master branch. We ought to choose explicitly what to test against. One approach: test against both master and the release branch, possibly not blocking on failures from master (since it's a moving target anyway, but at least we hear about breaking before it actually lands).

I ended up doing something like this with my golang binding to notmuch:

https://github.com/zenhack/go.notmuch/blob/master/.travis.yml

that's cool. I will copy some of that to one of my travis related branches and will see when to integrate it (this PR or later). But I want to use this approach definitely.

zenhack · 2016-12-10T05:45:24Z

test/test_database.py

+    def setUp(self):
+        self.corpus = Corpus()
+
+    def test_messagefile(self):


What is this test actually trying to verify? It doesn't actually invoke anything from alot.

pazz · 2016-12-10T09:27:09Z

That's nice as well! Maybe that could be a start for a separate travis integration, and a further patch can introduce the unit tests, and another PR can then integrate those into travis?

lucc · 2016-12-10T12:18:23Z

@pazz OK I tried to do that but it did not yet work. I was hoping that job 74 would pass and job 75 would fail but both passed.

I have trouble testing all this locally because the makefile in docs/ uses python but that is python3 on Arch Linux. I will try to fix that in a separate PR that should be merged first. It is here: #912

lucc · 2016-12-10T12:42:16Z

If we want to reduce the manual building of deps we can investigate travis-ci/travis-ci#5821

zenhack · 2016-12-10T17:29:18Z

Re: unexpected passes, sphinx-build has a -W that treats warnings as errors; we could make use of it.

lucc · 2016-12-11T01:22:12Z

@zenhack I have see that too and was planing to use it but my comment is more about git diff --exit-code not returning failure though I assumed there would be changes to commit after the makefile updated something. I hope I fixed that in #914. I will use -W in the travis file afterwards.

zenhack · 2016-12-11T04:32:43Z

Ah, I see -- I misinterpreted what you were referring to.

lucc · 2016-12-11T09:23:22Z

I managed to do it on top of #914. See build #89 and build #90.

What I did to simplify and speed things up is this: Install the simple dependencies with pip and mock the complicated ones. Then delete all generated files and have the makefile regenerate them. Then use git diff --exit-code to see if the commited versions were up to date.

I can put the commits from https://github.com/lucc/alot/tree/travis-check-docs-3 into this PR as soon as #914 is merged.

The question that remains is this: Do we only want to check if the generated doc sources are up to date (like in the other branch right now) or do we also want to build the complete documentation? I would then remove SPHINXBUILD=true from the make call (don't be fooled by this argument to make, it doesn't tell make to build the docs with sphinx but to use the true command in order to build it, so actually it doesn't build the html docs :)

Currently alot is only installed with pip and the docs are build. This can be expanded to also run the test as soon as we have some.

The alot command "exit" seems not to work as a "initial_command" value. So we use the forceful `os._exit` to kill the alot process. This still can show errors during startup (loading of the alot modules).

This also adds a first nontrivial test that checks if the notmuch bindings work as expected

pazz · 2016-12-12T09:08:13Z

@lucc (you didn't stick around long enough on irc for me to reply):
Generally, I prefer small PRs, so I would like to see one for introducing this travis file, maybe one or two "dummy" tests as individual patches, like the ones you have for testing if the docs build.
Maybe another patch that mentions travis in the docs, i.e., in the development section explains that we expect PRs to pass all tests etc.

I think adding a unit test suite should be another series, because what I wrote there was really preliminary and your travis stuff seems stable enough.
Also, conceptually these seem like different things: one contains (mostly) semantic tests around the code, the other is about running these (and others) on some server.

The only grey area is how/where to set up a test corpus. I tried to do something in this testsuite branch but its..well preliminary. But I think this should be done in some pre-hook of the test suite in order to run tests locally as well. If we put all this into the travis file then I see no easy way to test a PR before actually submitting it and waiting for travis.

lucc · 2016-12-14T11:51:29Z

This was superseded/resolved/fixed/done by #916 and #919. So I will close it now.

Three interesting things can be done later:

run tests against multible notmuch branches, see Add a travis file, prepare to run tests on travis-ci.org #886 (comment) (I have a branch ready for this but would wait for actual unit tests to be commited first)
only use containers and no VMs, this is blocked by APT whitelist request for libgmime-2.6-dev in trusty travis-ci/apt-package-safelist#3895, I have a branch ready to open a PR
add unit tests, discussed in Unit Tests #917

lucc force-pushed the travis branch from 04d7f42 to 17900b0 Compare July 17, 2016 13:53

lucc force-pushed the travis branch from 0b4c878 to 5b5f897 Compare July 20, 2016 08:22

lucc force-pushed the travis branch from 5b5f897 to 4f9ae23 Compare July 20, 2016 08:58

This was referenced Jul 20, 2016

Fix dependency specification in setup.py pazz/urwidtrees#28

Closed

Add option to handle servers with non-standard case-insensitive matching #887

Closed

lucc force-pushed the travis branch from 6384532 to 4f9ae23 Compare July 26, 2016 09:23

lucc mentioned this pull request Dec 4, 2016

Coordinating further development #908

Closed

lucc force-pushed the travis branch from 4f9ae23 to 185ee7a Compare December 9, 2016 12:56

ghost suggested changes Dec 9, 2016

View reviewed changes

zenhack reviewed Dec 10, 2016

View reviewed changes

lucc mentioned this pull request Dec 10, 2016

Fix building of docs to ease development #912

Merged

lucc added this to the release 0.5 milestone Dec 10, 2016

lucc force-pushed the travis branch from 185ee7a to f0ab513 Compare December 11, 2016 01:39

lucc and others added 11 commits December 11, 2016 21:19

Add a travis file

f8d38be

Currently alot is only installed with pip and the docs are build. This can be expanded to also run the test as soon as we have some.

Switch to container builds on travis

8e745e8

Add a minimal test to travis build

22424fc

The alot command "exit" seems not to work as a "initial_command" value. So we use the forceful `os._exit` to kill the alot process. This still can show errors during startup (loading of the alot modules).

Install notmuch before testing alot

009de11

Build notmuch from source

fdee6ec

use sudo again

badbaec

switch to trusty.

8fcaf76

Create an empty notmuch config file

3c43afc

introduce dummy unittest suite

a7e2be5

add testcorpus and utils to access it

033d3e9

This also adds a first nontrivial test that checks if the notmuch bindings work as expected

Run the test suite on travis

b4821f7

lucc force-pushed the travis branch from f0ab513 to b4821f7 Compare December 11, 2016 20:27

This was referenced Dec 12, 2016

Start using travis.ci to check auto generated docs #916

Merged

Set up several jobs to build docs and run tests in parallel #919

Merged

lucc closed this Dec 14, 2016

lucc deleted the travis branch December 15, 2016 11:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a travis file, prepare to run tests on travis-ci.org #886

Add a travis file, prepare to run tests on travis-ci.org #886

lucc commented Jul 17, 2016

lucc commented Jul 17, 2016

lucc commented Jul 18, 2016

pazz commented Jul 19, 2016

lucc commented Jul 19, 2016

pazz commented Jul 20, 2016

lucc commented Jul 20, 2016

ghost left a comment

ghost Dec 9, 2016

pazz commented Dec 9, 2016

lucc commented Dec 9, 2016

pazz commented Dec 9, 2016 via email

lucc commented Dec 9, 2016

zenhack Dec 10, 2016

lucc Dec 10, 2016

zenhack Dec 10, 2016

pazz commented Dec 10, 2016 via email

lucc commented Dec 10, 2016

lucc commented Dec 10, 2016

zenhack commented Dec 10, 2016

lucc commented Dec 11, 2016

zenhack commented Dec 11, 2016

lucc commented Dec 11, 2016

pazz commented Dec 12, 2016

lucc commented Dec 14, 2016

Add a travis file, prepare to run tests on travis-ci.org #886

Add a travis file, prepare to run tests on travis-ci.org #886

Conversation

lucc commented Jul 17, 2016

lucc commented Jul 17, 2016

lucc commented Jul 18, 2016

pazz commented Jul 19, 2016

lucc commented Jul 19, 2016

pazz commented Jul 20, 2016

lucc commented Jul 20, 2016

ghost left a comment

Choose a reason for hiding this comment

ghost Dec 9, 2016

Choose a reason for hiding this comment

pazz commented Dec 9, 2016

lucc commented Dec 9, 2016

pazz commented Dec 9, 2016 via email

lucc commented Dec 9, 2016

zenhack Dec 10, 2016

Choose a reason for hiding this comment

lucc Dec 10, 2016

Choose a reason for hiding this comment

zenhack Dec 10, 2016

Choose a reason for hiding this comment

pazz commented Dec 10, 2016 via email

lucc commented Dec 10, 2016

lucc commented Dec 10, 2016

zenhack commented Dec 10, 2016

lucc commented Dec 11, 2016

zenhack commented Dec 11, 2016

lucc commented Dec 11, 2016

pazz commented Dec 12, 2016

lucc commented Dec 14, 2016