Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Use Travis CI to test each change #249

Closed
jonas opened this Issue Mar 4, 2014 · 9 comments

Comments

Projects
None yet
4 participants
Owner

jonas commented Mar 4, 2014

It should build tig both with and without configure and run the tests.

Contributor

dmalikov commented Mar 5, 2014

Let me start with this https://github.com/dmalikov/tig/tree/feature/travis

language: c

install:
 - make && make install
 - make test

Currently tests are failing, thus travis badge is black.
Please provide some instructions about how to generate configure from configure.ac to use there.

Contributor

BenBergman commented Mar 5, 2014

That failing test is my fault and was left as a reminder of a proposed solution to an issue with the graph view code. I don't have time to implement a fix right now, so if it is getting in the way of CI, you can remove test/test-graph-samples/15_non_commit_lines.*

Contributor

lcd047 commented Mar 5, 2014

Ok, here's my (slightly fancier) attempt: https://github.com/lcd047/tig/compare

language: c

env:
    - TEST=autoconf
    - TEST=make

branches:
    only: travis

script:
    - if [ x$TEST = xautoconf ]; then ./autogen.sh && ./configure; fi
    - make && make test && sudo make install

notifications:
    recipients: lcd047@gmail.com
Contributor

dmalikov commented Mar 5, 2014

@lcd047 nice. Maybe it should be [[ "autoconf" == "$TEST" ]] but it is not so important.

I've not started pull request because of tig project specific urls. @jonas please merge @lcd047's branch as a first start and enable travis hook in the project preferences. This should be enough for the first time

Owner

jonas commented Mar 6, 2014

Great work @dmalikov and @lcd047 . I think it would be nice to also build the documentation, which would require adding:

before_install:
  - sudo apt-get update -qq
  - sudo apt-get install -qq --no-install-recommends asciidoc xmlto

and appending this to the build script:

    make install-doc-man install-doc-html

To be on the safe side maybe we should also apt-get install libncursesw5-dev.

Finally, before merging this I would prefer some rebased + squashed branch that I can integrate from.

Contributor

lcd047 commented Mar 6, 2014

Ok, this should be the final form: https://github.com/lcd047/tig/compare

language: c

env:
    - TEST=autoconf
    - TEST=make

branches:
    only: travis

before_install:
    - sudo apt-get update
    - sudo apt-get install --no-install-recommends asciidoc xmlto docbook-utils

script:
    - if [ x"$TEST" = xautoconf ]; then ./autogen.sh && ./configure; fi
    - make && make doc && make test && sudo make install install-doc

notifications:
    recipients: lcd047@gmail.com

To merge this, it's probably easier if you do it manually than by a pull request, since it involves some legwork between Travis and GitHub. You probably want to edit the email address for notifications, and the link for the Travis badge. You probably also want to test either all branches, or only the stable one.

Contributor

dmalikov commented Mar 6, 2014

@lcd047 take a look at PR #252

Contributor

lcd047 commented Mar 6, 2014

@dmalikov: libncursesw5-dev is not needed (it's already installed, or whatever), and your script doesn't build the manual.

Also, you probably shouldn't run the main build in the install stage. Not that it makes any difference in practice (all commands are still required to finish without error), but the purpose of that stage is to install prerequisites for the build. Building and installing the main program and the manual and running the tests should be the main script, because that is the main point of all this dance, the rest are just preparations.

Come to think of it, my script could be made a little more logical too, by moving apt-get install:

language: c

env:
    - TEST=autoconf
    - TEST=make

branches:
    only: travis

before_install: sudo apt-get update

install: sudo apt-get install --no-install-recommends asciidoc xmlto docbook-utils

script:
    - if [ x"$TEST" = xautoconf ]; then ./autogen.sh && ./configure; fi
    - make && make doc && make test && sudo make install install-doc

notifications:
    recipients: lcd047@gmail.com
Owner

jonas commented Mar 7, 2014

I've merged the PR to get the ball rolling. And with a small fix the build is now pushing. My focus moving forward will be to extend the test suite now that the code has been split into multiple files.

I'm not familiar with Travis best practice, so if it would be better to move things around in the Travis config please feel free to submit a new pull request.

@jonas jonas closed this Mar 7, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment