juju-release-tools

Merge lp://staging/~sinzui/juju-release-tools/upload-packges into lp://staging/juju-release-tools

  1. upload-packges
  2. Merge into trunk
Proposed by Curtis Hovey
Status: Merged
Merged at revision: 218
Proposed branch: lp://staging/~sinzui/juju-release-tools/upload-packges
Merge into: lp://staging/juju-release-tools
Diff against target: 220 lines (+211/-0)
2 files modified
tests/test_upload_packages.py (+119/-0)
upload_packages.py (+92/-0)
To merge this branch: bzr merge lp://staging/~sinzui/juju-release-tools/upload-packges
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Review via email: mp+269250@code.staging.launchpad.net

Description of the change

Upload source packages to Lp only when they are new.

This branch introduces a script that will dput source packages to a ppa if the archive doesn't have the source package. This script is a an old Lp test script I used to check if a source package is in an archive.

The only ssh-to-machine test in our releases is to do the dput. I didn't want to create a job that does an unchecked dput, because it can only be run once. This job can be run many times. if there is a network error while uploading, the job can be re-run to complete to dputs.

I intend to run this script like so

    PACKAGE_DIRS-$(ls -d $HOME/jobs/release-juju-source-packages/workspace/juju-build-*)
    juju-release-tools/upload_packages.py -c $JUJU_HOME/juju-qa-bot-lp-oauth.txt ppa:juju-packaging/$ppa $PACKAGE_DIRS

I want to solve the source package dir problem later (do not step into another job's workspace). This is more reliable that we have now.

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

The code looks reasonable. I have some style suggestions, especially in the tests.

I think you're using assert_called_with where you're expecting it to be called only once. I recommend assert_called_once_with instead.

In fact, I recommend avoiding assert_called_with(). I use:
# expecting 1 call
mock.assert_called_once_with('foo')

# Expecting more than one call, but a specific pattern:
self.assertEqual(mock.mock_calls, [call('foo'), call('bar')])

# Expecting no calls
self.assertEqual(mock.call_count, 0)

You have a pattern where you create a Mock and assign one of its attributes to Mock(). I think this is not needed; it is the default.

review: Approve
Revision history for this message
Curtis Hovey (sinzui) wrote :

As is said in IRC python's assertEquals's signature is (expected, actual) per the message shown in error. This is inconsistent with other unittest asserts and other implementations on unittest frameworks. I believe this is fixed in py3 http://bugs.python.org/issue10573.

I agree with your other remarks, though there is a complication with one inline that will deserve a comment to explain the oddity.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches