Merge lp:~hjd/widelands/tests-poc into lp:~widelands-dev/widelands/debian

Proposed by Hans Joachim Desserud
Status: Rejected
Rejected by: SirVer
Proposed branch: lp:~hjd/widelands/tests-poc
Merge into: lp:~widelands-dev/widelands/debian
Diff against target: 30 lines (+9/-0)
3 files modified
debian/control (+1/-0)
debian/tests/control (+2/-0)
debian/tests/testsuite (+6/-0)
To merge this branch: bzr merge lp:~hjd/widelands/tests-poc
Reviewer Review Type Date Requested Status
Widelands Developers Pending
Review via email: mp+250533@code.launchpad.net

Description of the change

I have some good news and bad news:
I have added support for running the integration/regression tests with the Debian package using autopkgtest. Unfortunately the Launchpad PPA builders doesn't support this [1]. :(

Autopkgtest, also known as DEP8 (see [2] for more info), is a way to test Debian packages in order to verify they run and behave as expected. The tests are run on binary packages which means you don't have to rebuild the package each time you want to run them. This makes it easy to run automated package tests more frequently, for instance Ubuntu runs them whenever they upload a new version or when one of its dependencies change [3].

I had read a bit about this already and knew that Ubuntu/Debian was working on adding test cases to some of their core packages to ensure quality and catch regressions easier. I hadn't really looked into it, but it turned out adding autopkgtests was a lot easier than I thought. All it really needed was a control file with the dependencies and one or more tests which can then be run. (Please ignore the early commits as they were mostly horrible hacks to see if I could get *something* to run on the PPA builders).

In order to verify this, I ran it on a local VM:
$ adt-run -B --built-tree=. --- null
I checked out the widelands source code and merged this branch into it to be able to build the Debian packages. It should be possible to run the tests when building the packages, but I wasn't able to trigger it. Since the main goal was getting the tests to run and compilation took a long time, I switched to using pre-built packages instead. I added my test-PPA to get the latest packages from trunk. The "null" at the end indicates I don't want to run the tests on my system instead of through QEMU, and LXC container or similar. The command above might not be the most optimal way of running things, but it worked as a proof of concept.

As mentioned above, the Launchpad PPA builders unfortunately won't run the tests. However, they will be run for offical Debian and Ubuntu packages so when build19 is packaged, they will be run to verify Widelands works on those systems. One issue remains on this front though, which I've mentioned before, is the need for a timeout mechanism. For now I've only enable one of the test suites, because when I tried to run all tests it would freeze or get stuck on one of them thus never exiting. Ideally this should run all tests, but it would need some way to mark these tests so it doesn't end up as a process which has to be killed manully.

Please let me know if something is unclear, or if you want more details, I wasn't sure how much to include.

[1] https://answers.launchpad.net/launchpad/+question/246354
[2] http://dep.debian.net/deps/dep8/
[3] http://packaging.ubuntu.com/html/auto-pkg-test.html#ubuntu-infrastructure

To post a comment you must log in.
Revision history for this message
SirVer (sirver) wrote :

Thanks for the explanations, very interesting and valuable experiments. Thanks for investing the time.

I am not sure how interesting it is to run our tests on a debian system on deploy. Do you think we should check that in? That is very late in the cycle of our product to catch bugs.

Otherwise I read that we cannot have automated nightly testing from our bzr branches - that is unfortunate and stays a big plus for Github.

Could you add a summary of your experiments to tinyurl.com/WlMoveToGithub with maybe a link here for more information.

As said, I am on the fence if this is worth merging. You have a better understanding of the implications, so I think you should decide.

Revision history for this message
Hans Joachim Desserud (hjd) wrote :
Download full text (4.4 KiB)

>I am not sure how interesting it is to run our tests on a debian system on deploy. Do you think we should check that in? That is very late in the cycle of our product to catch bugs.

(I'm not quite sure how to interpret this, are you thinking about the branch/build/package/deploy/run tests-cycle or release stable version-work-work-work-new release candidate-cycle? Let me know I'm responding to something completely different.)

My starting point for this experiment was the PPA builders which checks out and builds widelands in a clean environment, so that felt like a good starting point when attempting to add the regression test suite to the process. I didn't know exactly how to go about it, but the idea was to either run the tests on a recently-built binary just before it got packaged up or alternatively with the freshly made package. I couldn't get the former to work, but I added the package tests which I got running locally though unfortunately the PPA builders won't run them.

So the idea was to run these on each development build, but that seems tricky at least with Launchpad's current setup. However, support for running the tests has been added to the package so there is nothing blocking on our side.

For a small digression, my master plan on the Debian packaging is that it should be trivial to take the next release candidate we release together with our updated packaging and have the first set of packages ready on day 1. The majority of changes which would be necessary going from build18 to build19 is adding and removing build dependencies + adjusting cmake invocation.The Debian maintainer would have needed to do at some point anyways, but since we have continuously updated it throughout the development cycle, that work is already done. With the exception of Debian policies/guidelines we haven't heard of, that's the majority of the work needed to get the next Widelands release into Debian/Ubuntu/Linux Mint/etc already done.

Now, why is this relevant? Because if a package has tests, those tests will be run. I've triaged bugs in Ubuntu/Debian for some years now, and I've seen a surprisingly, even embarrassingly high number of packages where the main program either crash on startup or at the first sign of user interaction. (Mostly smaller, lesser-known programs, but still...) So I think even a smoketest which verifies the program starts and runs can be a useful gain.

More importantly, it can be used to verify the program still works after other changes in the system (this might be addressing your "on deploy" comment). Debian's CI system [1] states that the tests for a package will be run:
* when any package in the dependency chain of its binary packages changes;
* when the package itself changes;
* when 1 month is passed since the test suite was run for the last time.

For an example, let's say a new version of boost or SDL is packaged. In this case any dependent package which have tests will run them with the new version of the library. If Widelands for some reason fails with a newer version of this library, this problem can be discovered, pin-pointed and fixed immediately. Rather than the current situation where someone would report "...

Read more...

Revision history for this message
SirVer (sirver) wrote :

That answers all my questions I had.

> I do see that GitHub's ecosystem offers some nice possibilities.

What are you referring to here?

> PS. Yes, I'll add a note to the document. Maybe not today, but hey, I finally wrote a reply here. Is there a tentative deadline for adding thoughts and comments to that document, btw?

No, there is not. It is very clear in my mind that we will not do anything before b19, so we can continue gather knowledge for a while longer.

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

> we can continue gather knowledge for a while longer.
Ok, good, I feel like I haven't had time to look properly at all of it yet.

When talking about the ecosystem around GitHub I was mainly thinking about the Continuous Integration and similar services. That is really something we should have in place to keep track of when regressions are introduced, and being able to run it for branches prior to merging would probably help too.

Launchpad seems to only support daily builds (like we have now) and/or build on request. (And I managed to hit a daily request limit on my test PPA, so they're apparently not unlimited either). Which was a bit surprising to me since I was convinced LP recipies supported "build on each commit", but I was not able to find this anywhere.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I just happened across a project that used Jenkins with Lanuchpad. Maybe worth looking into?

https://code.launchpad.net/~nik90/ubuntu-clock-app/fix-translation-plural-forms/+merge/252838

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

GunChleoc: Interesting. I wonder how it is integrated with Launchpad and how the jobs are triggered. Also, I wonder whether that Jenkins instance would be open for other projects.

Revision history for this message
SirVer (sirver) wrote :

I hijacked the code review and asked. Maybe we get some information.

Revision history for this message
TiborB (tiborb95) wrote :

Seems useful.

In regard to these never-ending tests - I guess that problem are tests with fake mouse movements

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

Somewhat relevant: Launchpad has now officially announced beta support for git code hosting, see http://blog.launchpad.net/general/git-code-hosting-beta for details.

I saw this was discussed on reddit (https://www.reddit.com/r/Ubuntu/comments/34k8oi/git_code_hosting_beta_in_launchpad/) and one of the questions asked was regarding automated testing for merge proposal. The answer was that this should be fortcoming "reasonably soon". Travis was mentioned as an example (before anyone gets their hopes up), mostly as a "we-can't-guarantee-they-will-support-this". However, the possibility should be there, so let's hope Launchpad/Ubuntu and Travis talk together. :)

Revision history for this message
SirVer (sirver) wrote :

git is now rolled out on launchpad. I did not find anything about travis though - did you hjd?

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

> I did not find anything about travis though - did you hjd?

Not specifically, no.

I did see the following announcement (http://blog.launchpad.net/general/launchpad-news-august-2015) which mentioned webhook support has been added. (Though it points to a gmail bug, the real report might be bug 342729)

This is somewhat interesting because webhooks have been mentioned earlier as one of the big things which need to be in place. With the hooks it should be possible to pick up when a change happens in a repository and perform some action (start a build/testrun) based on it.

I guess nothing specifically has been mentioned about travis, since there is also a bit on their side whether they wish to expand to also support repositories on Launchpad.

As a side note, I can't find it right now, but they are working on tools/support for converting from an existing bzr repo to a git repo on Launchpad. They have started working on it for the Launchpad repo itself since they want to force themselves to deal with the edge cases they run into.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Hi, I am bunnybot (https://github.com/widelands/bunnybot).

I am keeping the source branch lp:~hjd/widelands/tests-poc mirrored to
  https://github.com/widelands/widelands/tree/_hjd_widelands_tests_poc

The latest continuous integration build can always be found here:
  https://travis-ci.org/widelands/widelands/branches
Please do not merge without making sure that it passes.

You can give me commands by starting a line with @bunnybot <command>. I understand:
 merge: Merges the source branch into the target branch, closing the pull request.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Travis build 124 has changed state to: failed. Details: https://travis-ci.org/widelands/widelands/builds/99784765.

Revision history for this message
Hans Joachim Desserud (hjd) wrote :

(This presumably failed to build because this branch isn't intended for merging into the main one, in case anyone were wondering)

Revision history for this message
bunnybot (widelandsofficial) wrote :

Travis build 124 has changed state to: failed. Details: https://travis-ci.org/widelands/widelands/builds/99784765.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Bunnybot encountered an error while working on this merge proposal:

Running 'git push github _hjd_widelands_tests_poc' failed. Output:

ssh: Could not resolve hostname github.com: Name or service not known
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

Revision history for this message
SirVer (sirver) wrote :

seems like a transient failure. If this gets too chatty I can special case these.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Bunnybot encountered an error while working on this merge proposal:

<urlopen error [Errno -2] Name or service not known>

Revision history for this message
bunnybot (widelandsofficial) wrote :

Travis build 124 has changed state to: failed. Details: https://travis-ci.org/widelands/widelands/builds/99784765.

Revision history for this message
SirVer (sirver) wrote :

I suggest closing this as we are running the regression test now for each merge proposal on travis via bunnybot. Do you agree, hjd?

Revision history for this message
bunnybot (widelandsofficial) wrote :

Travis build 124 has changed state to: failed. Details: https://travis-ci.org/widelands/widelands/builds/99784765.

Revision history for this message
SirVer (sirver) wrote :

I went ahead and put the current debian/ directory into trunk, so that we have an easier time keeping it up to date with changes to the codebase. I also deleted recipes and PPA for precise, which have not been build for > 1 year.

I adapted the recipe for widelands-daily to no longer merge the debian branch.

With these changes and bunnybot+travis+appveyor doing CI for us I think this branch is no longer required. Setting to rejected.

I will also set the debian branch to merged, since it sorta is (with loss of history though).

Unmerged revisions

40. By Hans Joachim Desserud

Add comment on why the tests are restricted to a single suite

39. By Hans Joachim Desserud

Remove added test which probably didn't have any effect anyways

38. By Hans Joachim Desserud

Reorder and make the testsuite executable

37. By Hans Joachim Desserud

Maybe I need to override the test step to trigger them to run?

36. By Hans Joachim Desserud

Attempt to add a proper autopkgtest

35. By Hans Joachim Desserud

Now try to run it after actually building something

34. By Hans Joachim Desserud

Specify widelands binary

33. By Hans Joachim Desserud

singular

32. By Hans Joachim Desserud

Attempt to run the script

31. By Hans Joachim Desserud

comma

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'debian/control'
--- debian/control 2014-12-20 17:52:44 +0000
+++ debian/control 2015-02-21 15:43:19 +0000
@@ -22,6 +22,7 @@
22 libglew1.6-dev | libglew-dev,22 libglew1.6-dev | libglew-dev,
23 libpng-dev, 23 libpng-dev,
24 zlib1g-dev24 zlib1g-dev
25XS-Testsuite: autopkgtest
25Vcs-Git: git://git.debian.org/git/pkg-games/widelands.git26Vcs-Git: git://git.debian.org/git/pkg-games/widelands.git
26Vcs-Browser: http://anonscm.debian.org/gitweb/?p=pkg-games/widelands.git27Vcs-Browser: http://anonscm.debian.org/gitweb/?p=pkg-games/widelands.git
27#Vcs-Svn: svn://svn.debian.org/svn/collab-maint/deb-maint/widelands/ <-- Old location of packaging28#Vcs-Svn: svn://svn.debian.org/svn/collab-maint/deb-maint/widelands/ <-- Old location of packaging
2829
=== added directory 'debian/tests'
=== added file 'debian/tests/control'
--- debian/tests/control 1970-01-01 00:00:00 +0000
+++ debian/tests/control 2015-02-21 15:43:19 +0000
@@ -0,0 +1,2 @@
1Tests: testsuite
2Depends: @, xvfb
03
=== added file 'debian/tests/testsuite'
--- debian/tests/testsuite 1970-01-01 00:00:00 +0000
+++ debian/tests/testsuite 2015-02-21 15:43:19 +0000
@@ -0,0 +1,6 @@
1#!/bin/sh
2# Right now it only runs the lua_testsuite.
3# It can (and should) be expanded to run all tests, but when I tried
4# I ran in to situations where some of the tests would never finish
5xvfb-run -a --server-args="-screen 0 1024x768x24" \
6./regression_test.py -b widelands -r lua_testsuite

Subscribers

People subscribed via source and target branches