Merge lp:~pwlars/uci-engine/tr-pass-no-tests into lp:uci-engine

[Paul Larson]

Don't fail the test run if there are no tests defined in the package.

I can't seem to get a deploy to work in lxc at the moment. Not sure why yet but I'll have to debug in the morning. I think this should make it so that the test runner doesn't fail in the event that we run a ticket through the system that happens to contain no tests.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:741
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1267/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1267/rebuild

review: Approve (continuous-integration)

[Celso Providelo (cprov)]

Paul,

Thanks a lot for investigating this issue and finding a reasonable solution.

I, personally, agree with "passing" sources that have no tests in the ci-airline context. However, it has to make sense to our audience as well. Can we have another *representative* opinion on that ?

Meanwhile, can you add a quick unittest (mocking test_package, possibly) documenting/checking this behaviour ? This way we might notice future changes (regressions) like this one.

review: Needs Fixing

[lp:~pwlars/uci-engine/tr-pass-no-tests]

742. By Paul Larson on 2014-08-12

resolve merge conflicts

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:742
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1271/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1271/rebuild

review: Approve (continuous-integration)

[Ubuntu CI Bot (uci-bot)]

We discussed going ahead and landing this in the call this morning. I was blocked on the tests because they didn't work right, and now blocked on getting deployment working with other dependencies on a (now worked around) change to make booting nova instances work with the new cloud we use. So there are a lot of things making the tests difficult for what would be an otherwise obvious change. I'll talk to vila when he gets back and once we get the other things sorted out I can modify the tests so that they expect the new behavior. It's just not easy to actually see them work right now.

[Ursula Junque (ursinha)]

According to pitti the "no tests found" happens often when there is a packaging bug, so a real failure. We'll need to a way around this so the system can be informed the package knowingly has no tests and therefore test runner shouldn't fail.

<Ursinha> pitti: hey :) I have a question about autopkgtest, more specifically adt-run, that you might have the answer: when a package has no tests adt-run return code is 8, is that only to create a distinction between tests that actually ran and passed and no tests found? Or is there the intention to consider lack of tests a failure?
<pitti> Ursinha: yes, we usually consider 8 a failure as way more often than not it's an unintended packaging bug

[Evan (ev)]

Ursula, will we have the same concern as Martin that we don't want to silently let a packaging bug through?

Could we arrive somewhere in the middle by making this an error if autopkgtest returns 8 and debian/tests exists? Martin's case of packaging error is seemingly covered, as is our desire to not error when there hasn't been any movement towards DEP8 in that package.

Thoughts?

[Paul Larson (pwlars)]

After further discussion, we decided this is a premature optimization. The intent of adt-run was always to fail when there are no tests in the package. If someone has a compelling reason why we shouldn't do this, then we can discuss further, but the only thing that comes to mind is that some packages may be simple enough to not need tests. I don't see any reason why we couldn't push back that an intentionally simple test could be added in this case. So... abandoning this for now unless someone has a good reason to do it.

Unmerged revisions

742. By Paul Larson on 2014-08-12

resolve merge conflicts

741. By Paul Larson on 2014-08-11

Pass when there are no tests in the package

=== modified file 'test_runner/tstrun/run_worker.py'
--- test_runner/tstrun/run_worker.py	2014-08-06 14:06:52 +0000
+++ test_runner/tstrun/run_worker.py	2014-08-12 23:12:51 +0000
@@ -245,9 +245,11 @@
                                              data_store, ticket_id,
                                              self.logger_name)
             return_code, artifacts = req_handler.handle(params, package)
-            # 0 is success, 2 means "at least one test skipped" and is
-            # considered a success
-            if return_code not in (0, 2):
+            # The following return codes are considered passing:
+            # 0 is success
+            # 2 means "at least one test skipped"
+            # 8 means "no tests in this package"
+            if return_code not in (0, 2, 8):
                 # At least one test failed
                 notify = amqp_utils.progress_failed
             results['artifacts'].extend(artifacts)