Skip to content

Add a test suite.#194

Merged
rkitover merged 7 commits intomasterfrom
test
Jun 26, 2016
Merged

Add a test suite.#194
rkitover merged 7 commits intomasterfrom
test

Conversation

@lucc
Copy link
Copy Markdown
Collaborator

@lucc lucc commented Jun 9, 2016

This is an attempt to finish #180.

When reviewing #193 it suddenly came to me that script could be used to capture the help and version output at least. Like this: script -c './vimpager -h' -e -q. This pattern is used with the bats test framework.

The decision between bats and roundup is rather arbitrary. I went ahead and used bats because roundup is not available in the Arch repos and that is what I use vor development. If that is unadaptable I can also convert these tests to another test suite.

PS: I don't really understand why travis fails to install bats. Can we somehow force the build to restart without pushing to this branch?

@lucc
Copy link
Copy Markdown
Collaborator Author

lucc commented Jun 16, 2016

I managed to install bats but the git tags of the vimpager repository are not available on travis and they are needed to get the correct version output.

@rkitover any ideas?

rkitover added a commit that referenced this pull request Jun 18, 2016
Add a comment to the top of vimpager and vimcat with the current
release version, and use it in --version output when git describe has
failed.
@rkitover
Copy link
Copy Markdown
Owner

@lucc sorry things have been difficult here, but I'm slowly getting back into things.

Does the above commit help you?

rkitover added a commit that referenced this pull request Jun 18, 2016
Adjust valid version glob pattern from `[0-9]*` to `[0-9].`, this makes
sure that git describe picked up an actual tag.
@rkitover
Copy link
Copy Markdown
Owner

@lucc made a minor adjustment to make sure a tag is picked up above.

@lucc
Copy link
Copy Markdown
Collaborator Author

lucc commented Jun 18, 2016

@rkitover I don't really like the solution. If we really need to supply a fallback version number I would suggest something like 5438492. (But the makefile is not yet updated with that commit.)

In my opinion it would be better to solve this issue on Travis. Like this we introduce additional code paths just for the test. That means we don't test the normal code paths at all.

@lucc
Copy link
Copy Markdown
Collaborator Author

lucc commented Jun 18, 2016

Also: The first question we should answer here is if we want to use bats and keep these tests? Otherwise we have to decide a new test framework and re-implement these tests.

@rkitover
Copy link
Copy Markdown
Owner

@lucc check this out: https://travis-ci.org/rkitover/vimpager/builds/138622907

so all you need to do is stick git fetch --unshallow somehwere in the script:.

re: 5438492

  • I would still like the last release tag somewhere at the top of the file, it can be a variable assignment instead of a comment as well, that's fine. That's even better.
  • Of course it would be nice for the makefile to update the release tag, but also fine without this, since we can update it in the release commit.
  • I see now that git describe works ONLY with tags, so some of my code was definitely unnecessary.
  • Don't call it (fallback) something like (checkout) would be better.

re: this PR

I haven't had a chance to look at the code yet, BATS is what I wanted to use, so that's good, I'll try to read the commits later today. I'm dealing with an eye infection, back and knee pain and lots of other horrible shit right now.

@lucc lucc force-pushed the test branch 4 times, most recently from 4b3714f to dd83f2b Compare June 20, 2016 04:24
@lucc
Copy link
Copy Markdown
Collaborator Author

lucc commented Jun 20, 2016

I think I fixed the travis build. The core problem was that make install-deb did internally run our make test so I had to move it to the script: step in .travis.yml.

Please do not merge yet, I am still cleaning up the git history and testing more stuff.

@lucc lucc force-pushed the test branch 6 times, most recently from 8bfcdba to 50f8c2e Compare June 20, 2016 09:21
@lucc lucc mentioned this pull request Jun 20, 2016
@lucc lucc force-pushed the test branch 5 times, most recently from d210bc3 to 87d4810 Compare June 20, 2016 23:09
@lucc
Copy link
Copy Markdown
Collaborator Author

lucc commented Jun 20, 2016

@rkitover The history is still not clean but that is only because there is a question that came up: The file debian/source/lintian-overrides tells lintian to ignore newer standards versions but we still update it when building on travis.

We could either replace the call to scripts/update_lintian with sudo apt-get update in the travis file or remove this lintian override. I tested both and the build on travis pass.

What do you think? Are the new version of lintian or the override needed for some other reason?

Other than that and adding however many tests this PR is done (from my point of view).

@lucc
Copy link
Copy Markdown
Collaborator Author

lucc commented Jun 21, 2016

I also pushed https://github.com/rkitover/vimpager/tree/test2 . I will open a PR for that as soon as this one is merged, as it is a child of this one. But you can already have a look.

@lucc lucc force-pushed the test branch 2 times, most recently from 742e8eb to b383415 Compare June 22, 2016 07:51
rkitover added a commit that referenced this pull request Jun 23, 2016
Add support for `make install-deb CLEAN_BUILD_DEPS=0` to not run
`apt-get autoremove` so that build dependencies are not removed.
@rkitover
Copy link
Copy Markdown
Owner

@lucc the reason for update_lintian is to use the latest version of lintian, the travis VM is trusty which is old.

If the new lintian is complaining that the standards version is too old, we should update the standards version in debian/ . If there are incompatibilies with the latest standards version then I will fix them. Also why am I not seeing this issue on master?

I added support for not removing build deps in make install-deb above, while debhelper has support for not running tests, so change the sudo make install-deb command in .travis.yml to:

sudo make install-deb CLEAN_BUILD_DEPS=0 DEB_BUILD_OPTIONS=nocheck

then you can run make test after the debian package is installed and the build deps will not have been removed and debhelper will not run make test. This will make manually installing sharutils unnecessary.

lucc added 6 commits June 24, 2016 00:38
- Move `make install-deb` to the script step as it runs the tests.
- Install uuencode (package sharutils) explicitly as it is needed during
  testing.  Formally it was implicitly installed by `make install-deb`.
- Install bats manually as there is no package for it on trusty.
- Remove bats building directory as it is reported by lintian.
- Fetch git tags as they are needed to run the tests.
- Run `make all`.
- Add a modeline.
@lucc
Copy link
Copy Markdown
Collaborator Author

lucc commented Jun 23, 2016

Thanks for the info. I added that and do now consider ths PR done. Feel free to merge if you don't have further comments.

@rkitover
Copy link
Copy Markdown
Owner

@lucc finally read the actual tests, awesome stuff! Merging.

@rkitover rkitover merged commit 26745c7 into master Jun 26, 2016
@rkitover rkitover deleted the test branch June 26, 2016 12:43
rkitover added a commit that referenced this pull request Jun 26, 2016
Replace script -q -e -c invocation in bats tests with the run_cmd
helper which uses a more portable invocation of script(1) which works
on OS X, Ubuntu and OpenBSD.
@rkitover
Copy link
Copy Markdown
Owner

@lucc I made some changes to call script(1) in a more portable way.

Tests now pass on OS X, Ubuntu, OpenBSD and FreeBSD.

@lucc
Copy link
Copy Markdown
Collaborator Author

lucc commented Jun 29, 2016

Nice, I opend #196 as a followup.

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

Successfully merging this pull request may close these issues.

2 participants