Ubuntu Test Cases

Merge lp://staging/~canonical-platform-qa/ubuntu-test-cases/healthcheck-to-dep8 into lp://staging/ubuntu-test-cases/touch

healthcheck-to-dep8
Merge into touch

Proposed by Richard Huddie on 2014-11-06

Status:	Merged
Merge reported by:	Christopher Lee
Merged at revision:	not available
Proposed branch:	lp://staging/~canonical-platform-qa/ubuntu-test-cases/healthcheck-to-dep8
Merge into:	lp://staging/ubuntu-test-cases/touch
Diff against target:	250 lines (+144/-18) 7 files modified tests/health-check/debian/changelog (+5/-0) tests/health-check/debian/control (+16/-0) tests/health-check/debian/source/format (+1/-0) tests/health-check/debian/tests/control (+3/-0) tests/health-check/debian/tests/health-check (+18/-0) tests/health-check/health-check-test-pid.py (+16/-18) tests/health-check/health-check-test-run-pids.py (+85/-0)
To merge this branch:	bzr merge lp://staging/~canonical-platform-qa/ubuntu-test-cases/healthcheck-to-dep8
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Christopher Lee (community)		2014-11-06	Approve on 2014-11-13
Review via email: mp+240925@code.staging.launchpad.net

Commit message

Port health-check tests to use dep8 for autopkgtest.

Description of the change

Port health-check tests to use dep8 for autopkgtest.

Revision history for this message

Christopher Lee (veebers) wrote on 2014-11-07:

Hi Richard, looking good. Just a couple of things to fix / sort out.

Remind me, do you intend to remove the un-needed stuff from debian/*? I'm sure you said that some of it wasn't needed in this context.

review: Needs Fixing

lp://staging/~canonical-platform-qa/ubuntu-test-cases/healthcheck-to-dep8 updated on 2014-11-07

342. By Richard Huddie on 2014-11-07: review comments for pids array and subprocess logging
343. By Richard Huddie on 2014-11-07: use params for procmap file and test script. Use instead of /tmp/ path
344. By Richard Huddie on 2014-11-07: use print() instead of sys.stdout.write()
345. By Richard Huddie on 2014-11-07: remove un-neccessary debian files
346. By Richard Huddie on 2014-11-07: update changelog

Revision history for this message

Richard Huddie (rhuddie) wrote on 2014-11-07:

> Hi Richard, looking good. Just a couple of things to fix / sort out.
>
> Remind me, do you intend to remove the un-needed stuff from debian/*? I'm sure
> you said that some of it wasn't needed in this context.

Hi Chris,

Thanks for your review and comments!

I've updated accordingly for all your comments and added some reply comments. I've removed as many debian files as I can, but debian/control and debian/changelog are necessary.

Let me know if you think anything else needs doing.

Cheers.

Revision history for this message

Christopher Lee (veebers) wrote on 2014-11-10:

Awesome, looking good Richard. Just 2 things, 1. I don't see any replies? 2. It looks like all of your print statements have extra parans (i.e. diff line 82: print(("string"))).

Other than the double parans this looks ace.

review: Needs Fixing

Revision history for this message

Christopher Lee (veebers) wrote on 2014-11-10:

Oops, missed one thing with the Vcs-branch (inline comment) which will need clarification.

review: Needs Fixing

lp://staging/~canonical-platform-qa/ubuntu-test-cases/healthcheck-to-dep8 updated on 2014-11-10

347. By Richard Huddie on 2014-11-10: Remove un-neccessary parenthesis in print(), and debian/control fields

Revision history for this message

Richard Huddie (rhuddie) wrote on 2014-11-10:

Thanks Chris. I removed those lines in debian/control as the test runs fine without them.

Regarding the print() statements, I also removed the extra brackets. My python editor warned me that they are needed for python3 support, but it works fine without.

Revision history for this message

Christopher Lee (veebers) wrote on 2014-11-11:

Just one more thing since Paul clarified this (https://code.launchpad.net/~canonical-platform-qa/ubuntu-test-cases/sample-adt-test/+merge/240974/comments/593446) the debian folder should go in tests/health-check.
An example of this is here: https://code.launchpad.net/~canonical-platform-qa/ubuntu-test-cases/memevent-dep8/+merge/241378