Merge lp:~wgrant/launchpad/test-ddeb-matching into lp:launchpad/db-devel
- test-ddeb-matching
- Merge into db-devel
Proposed by
William Grant
| Status: | Merged |
|---|---|
| Approved by: | Māris Fogels |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 9622 |
| Proposed branch: | lp:~wgrant/launchpad/test-ddeb-matching |
| Merge into: | lp:launchpad/db-devel |
| Prerequisite: | lp:~wgrant/launchpad/refactor-nuf-creation |
| Diff against target: |
962 lines (+226/-114) 17 files modified
lib/canonical/launchpad/testing/fakepackager.py (+2/-1) lib/lp/archiveuploader/nascentupload.py (+70/-52) lib/lp/archiveuploader/tests/nascentupload-announcements.txt (+20/-19) lib/lp/archiveuploader/tests/nascentupload-packageset.txt (+3/-3) lib/lp/archiveuploader/tests/nascentupload-security-uploads.txt (+2/-2) lib/lp/archiveuploader/tests/nascentupload.txt (+20/-19) lib/lp/archiveuploader/tests/test_nascentupload.py (+85/-0) lib/lp/archiveuploader/tests/test_nascentupload_documentation.py (+8/-3) lib/lp/archiveuploader/uploadprocessor.py (+2/-1) lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt (+1/-1) lib/lp/soyuz/doc/build-failedtoupload-workflow.txt (+1/-1) lib/lp/soyuz/doc/distroseriesqueue-ddtp-tarball.txt (+3/-3) lib/lp/soyuz/doc/distroseriesqueue-debian-installer.txt (+1/-1) lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt (+4/-4) lib/lp/soyuz/doc/distroseriesqueue-translations.txt (+1/-1) lib/lp/soyuz/doc/distroseriesqueue.txt (+2/-2) lib/lp/soyuz/scripts/tests/test_queue.py (+1/-1) |
| To merge this branch: | bzr merge lp:~wgrant/launchpad/test-ddeb-matching |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Māris Fogels (community) | code | Approve | |
|
Review via email:
|
|||
Commit message
Add unit tests for NascentUpload's DDEB matching code.
Description of the change
This branch factors out and adds unit tests for the DDEB-matching part of NascentUpload.
To make the tests less foul, I've altered NascentUpload to allow passing in an existing ChangesFile-like object, rather than requiring a path to a real file. This should eventually let us shrink existing tests throughout archiveuploader, and even remove test package data.
I've left the existing basic end-to-end test at the end of nascentupload-
To post a comment you must log in.
Revision history for this message
| William Grant (wgrant) wrote : | # |
> Hi William,
>
> This is a meaty change, lots of new code. Very nice. I especially like the
> refactoring of NascentUpload and _matchDDEBs(): both excellent changes.
>
> Here is the list of notes I took while reviewing the code:
>
> • Do ChangesFile objects have a .path attribute that you can use instead of
> string-sniffing in the constructor? If so, then perhaps you can push the
> fiddly conditional construction down into ChangesFile itself and at the same
> time eliminate the .changesfile_path attribute from the NascentUpload object.
> A NascentUpload.
> the conditional construction entirely.
Both fixed. All path-based NascentUpload() callsites updated to use NascentUpload.
> • Switching from .reject() to 'yield UploadError()' is a pretty big API
> change, however I do not see any code in the diff (besides the tests) that is
> impacted by it. Why not?
It's a pattern used throughout archiveuploader -- we call
run_and_
yielded errors.
> • It would be good to have a docstring for _matchDDEBs() that tells you that
> it is a generator method.
Missed that. Fixed.
> • I think the test names could be improved a bit: the comment should be used
> as test method name, because the comments tell you what the desired behaviour
> is. They tell you what the test is actually testing.
Fair point. Fixed.
> • You might be able to rewrite all those "self.assertEqu
> list(self.
> testtools.
> matchesDDEBs([]))
I opted to instead create a tiny domain-specific assertion method.
> With the above points taken into consideration, r=mars
Thanks.
Revision history for this message
| Māris Fogels (mars) wrote : | # |
These changes still look good to me. I'll run it through ec2 land.
Maris
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
| 1 | === modified file 'lib/canonical/launchpad/testing/fakepackager.py' |
| 2 | --- lib/canonical/launchpad/testing/fakepackager.py 2009-09-14 04:07:18 +0000 |
| 3 | +++ lib/canonical/launchpad/testing/fakepackager.py 2010-08-04 22:12:18 +0000 |
| 4 | @@ -278,7 +278,8 @@ |
| 5 | if suite is not None: |
| 6 | policy.setDistroSeriesAndPocket(suite) |
| 7 | |
| 8 | - upload = NascentUpload(changesfile_path, policy, logger) |
| 9 | + upload = NascentUpload.from_changesfile_path( |
| 10 | + changesfile_path, policy, logger) |
| 11 | upload.process() |
| 12 | |
| 13 | return upload |
| 14 | |
| 15 | === modified file 'lib/lp/archiveuploader/nascentupload.py' |
| 16 | --- lib/lp/archiveuploader/nascentupload.py 2010-08-03 08:49:19 +0000 |
| 17 | +++ lib/lp/archiveuploader/nascentupload.py 2010-08-04 22:12:18 +0000 |
| 18 | @@ -86,24 +86,35 @@ |
| 19 | # Defined if we successfully do_accept() and storeObjectsInDatabase() |
| 20 | queue_root = None |
| 21 | |
| 22 | - def __init__(self, changesfile_path, policy, logger): |
| 23 | - """Setup a ChangesFile based on given changesfile path. |
| 24 | + def __init__(self, changesfile, policy, logger): |
| 25 | + """Setup a NascentUpload for the given ChangesFile. |
| 26 | |
| 27 | - May raise FatalUploadError due to unrecoverable problems building |
| 28 | - the ChangesFile object. |
| 29 | - Also store given and initialized Upload Policy, as 'policy' |
| 30 | + :param changesfile: the ChangesFile object to be uploaded. |
| 31 | + :param policy: the upload policy to be used. |
| 32 | + :param logger: the logger to be used. |
| 33 | """ |
| 34 | - self.changesfile_path = changesfile_path |
| 35 | self.policy = policy |
| 36 | self.logger = logger |
| 37 | |
| 38 | self.rejections = [] |
| 39 | self.warnings = [] |
| 40 | - |
| 41 | self.librarian = getUtility(ILibraryFileAliasSet) |
| 42 | + |
| 43 | + self.changes = changesfile |
| 44 | + |
| 45 | + @classmethod |
| 46 | + def from_changesfile_path(cls, changesfile_path, policy, logger): |
| 47 | + """Create a NascentUpload from the given changesfile path. |
| 48 | + |
| 49 | + May raise FatalUploadError due to unrecoverable problems building |
| 50 | + the ChangesFile object. |
| 51 | + |
| 52 | + :param changesfile_path: path to the changesfile to be uploaded. |
| 53 | + :param policy: the upload policy to be used. |
| 54 | + :param logger: the logger to be used. |
| 55 | + """ |
| 56 | try: |
| 57 | - self.changes = ChangesFile( |
| 58 | - changesfile_path, self.policy, self.logger) |
| 59 | + changesfile = ChangesFile(changesfile_path, policy, logger) |
| 60 | except UploadError, e: |
| 61 | # We can't run reject() because unfortunately we don't have |
| 62 | # the address of the uploader to notify -- we broke in that |
| 63 | @@ -112,6 +123,7 @@ |
| 64 | # rejection to the archive admins. For now, this will end |
| 65 | # up in the script log. |
| 66 | raise FatalUploadError(str(e)) |
| 67 | + return cls(changesfile, policy, logger) |
| 68 | |
| 69 | def process(self): |
| 70 | """Process this upload, checking it against policy, loading it into |
| 71 | @@ -148,6 +160,7 @@ |
| 72 | self._check_sourceful_consistency() |
| 73 | if self.binaryful: |
| 74 | self._check_binaryful_consistency() |
| 75 | + self.run_and_collect_errors(self._matchDDEBs) |
| 76 | |
| 77 | self.run_and_collect_errors(self.changes.verify) |
| 78 | |
| 79 | @@ -155,35 +168,6 @@ |
| 80 | for uploaded_file in self.changes.files: |
| 81 | self.run_and_collect_errors(uploaded_file.verify) |
| 82 | |
| 83 | - unmatched_ddebs = {} |
| 84 | - for uploaded_file in self.changes.files: |
| 85 | - if isinstance(uploaded_file, DdebBinaryUploadFile): |
| 86 | - ddeb_key = (uploaded_file.package, uploaded_file.version, |
| 87 | - uploaded_file.architecture) |
| 88 | - if ddeb_key in unmatched_ddebs: |
| 89 | - self.reject("Duplicated debug packages: %s %s (%s)" % |
| 90 | - ddeb_key) |
| 91 | - else: |
| 92 | - unmatched_ddebs[ddeb_key] = uploaded_file |
| 93 | - |
| 94 | - for uploaded_file in self.changes.files: |
| 95 | - # We need exactly a DEB, not a DDEB. |
| 96 | - if (isinstance(uploaded_file, DebBinaryUploadFile) and |
| 97 | - not isinstance(uploaded_file, DdebBinaryUploadFile)): |
| 98 | - try: |
| 99 | - matching_ddeb = unmatched_ddebs.pop( |
| 100 | - (uploaded_file.package + '-dbgsym', |
| 101 | - uploaded_file.version, |
| 102 | - uploaded_file.architecture)) |
| 103 | - except KeyError: |
| 104 | - continue |
| 105 | - uploaded_file.ddeb_file = matching_ddeb |
| 106 | - matching_ddeb.deb_file = uploaded_file |
| 107 | - |
| 108 | - if len(unmatched_ddebs) > 0: |
| 109 | - self.reject("Orphaned debug packages: %s" % ', '.join('%s %s (%s)' % d |
| 110 | - for d in unmatched_ddebs)) |
| 111 | - |
| 112 | if (len(self.changes.files) == 1 and |
| 113 | isinstance(self.changes.files[0], CustomUploadFile)): |
| 114 | self.logger.debug("Single Custom Upload detected.") |
| 115 | @@ -197,7 +181,7 @@ |
| 116 | "Upload rejected because it contains binary packages.", |
| 117 | "Ensure you are using `debuild -S`, or an equivalent", |
| 118 | "command, to generate only the source package before", |
| 119 | - "re-uploading." |
| 120 | + "re-uploading.", |
| 121 | ] |
| 122 | if self.is_ppa: |
| 123 | messages.append( |
| 124 | @@ -241,7 +225,7 @@ |
| 125 | @property |
| 126 | def filename(self): |
| 127 | """Return the changesfile name.""" |
| 128 | - return os.path.basename(self.changesfile_path) |
| 129 | + return os.path.basename(self.changesfile.filepath) |
| 130 | |
| 131 | @property |
| 132 | def is_new(self): |
| 133 | @@ -254,7 +238,6 @@ |
| 134 | # |
| 135 | # Overall consistency checks |
| 136 | # |
| 137 | - |
| 138 | def _check_overall_consistency(self): |
| 139 | """Heuristics checks on upload contents and declared architecture. |
| 140 | |
| 141 | @@ -329,7 +312,8 @@ |
| 142 | """Heuristic checks on a sourceful upload. |
| 143 | |
| 144 | Raises AssertionError when called for a non-sourceful upload. |
| 145 | - Ensures a sourceful upload has exactly one DSC. |
| 146 | + Ensures a sourceful upload has exactly one DSC. All further source |
| 147 | + checks are performed later by the DSC. |
| 148 | """ |
| 149 | assert self.sourceful, ( |
| 150 | "Source consistency check called for a non-source upload") |
| 151 | @@ -369,10 +353,49 @@ |
| 152 | if len(considered_archs) > max: |
| 153 | self.reject("Upload has more architetures than it is supported.") |
| 154 | |
| 155 | + def _matchDDEBs(self): |
| 156 | + """Check and link DEBs and DDEBs in the upload. |
| 157 | + |
| 158 | + Matches each DDEB to its corresponding DEB, adding links in both |
| 159 | + directions. Unmatched or duplicated DDEBs result in upload errors. |
| 160 | + |
| 161 | + This method is an error generator, i.e, it returns an iterator over |
| 162 | + all exceptions that are generated while processing all mentioned |
| 163 | + files. |
| 164 | + """ |
| 165 | + unmatched_ddebs = {} |
| 166 | + for uploaded_file in self.changes.files: |
| 167 | + if isinstance(uploaded_file, DdebBinaryUploadFile): |
| 168 | + ddeb_key = (uploaded_file.package, uploaded_file.version, |
| 169 | + uploaded_file.architecture) |
| 170 | + if ddeb_key in unmatched_ddebs: |
| 171 | + yield UploadError( |
| 172 | + "Duplicated debug packages: %s %s (%s)" % ddeb_key) |
| 173 | + else: |
| 174 | + unmatched_ddebs[ddeb_key] = uploaded_file |
| 175 | + |
| 176 | + for uploaded_file in self.changes.files: |
| 177 | + # We need exactly a DEB, not a DDEB. |
| 178 | + if (isinstance(uploaded_file, DebBinaryUploadFile) and |
| 179 | + not isinstance(uploaded_file, DdebBinaryUploadFile)): |
| 180 | + try: |
| 181 | + matching_ddeb = unmatched_ddebs.pop( |
| 182 | + (uploaded_file.package + '-dbgsym', |
| 183 | + uploaded_file.version, |
| 184 | + uploaded_file.architecture)) |
| 185 | + except KeyError: |
| 186 | + continue |
| 187 | + uploaded_file.ddeb_file = matching_ddeb |
| 188 | + matching_ddeb.deb_file = uploaded_file |
| 189 | + |
| 190 | + if len(unmatched_ddebs) > 0: |
| 191 | + yield UploadError( |
| 192 | + "Orphaned debug packages: %s" % ', '.join( |
| 193 | + '%s %s (%s)' % d for d in unmatched_ddebs)) |
| 194 | + |
| 195 | # |
| 196 | # Helpers for warnings and rejections |
| 197 | # |
| 198 | - |
| 199 | def run_and_check_error(self, callable): |
| 200 | """Run the given callable and process errors and warnings. |
| 201 | |
| 202 | @@ -471,7 +494,6 @@ |
| 203 | # |
| 204 | # Signature and ACL stuff |
| 205 | # |
| 206 | - |
| 207 | def verify_acl(self): |
| 208 | """Check the signer's upload rights. |
| 209 | |
| 210 | @@ -501,7 +523,7 @@ |
| 211 | ISourcePackageNameSet).queryByName(self.changes.dsc.package) |
| 212 | |
| 213 | rejection_reason = archive.checkUpload( |
| 214 | - uploader, self.policy.distroseries, source_name, |
| 215 | + uploader, self.policy.distroseries, source_name, |
| 216 | self.changes.dsc.component, self.policy.pocket, not self.is_new) |
| 217 | |
| 218 | if rejection_reason is not None: |
| 219 | @@ -510,7 +532,6 @@ |
| 220 | # |
| 221 | # Handling checking of versions and overrides |
| 222 | # |
| 223 | - |
| 224 | def getSourceAncestry(self, uploaded_file): |
| 225 | """Return the last published source (ancestry) for a given file. |
| 226 | |
| 227 | @@ -739,8 +760,8 @@ |
| 228 | return |
| 229 | |
| 230 | component_override_map = { |
| 231 | - 'contrib' : 'multiverse', |
| 232 | - 'non-free' : 'multiverse', |
| 233 | + 'contrib': 'multiverse', |
| 234 | + 'non-free': 'multiverse', |
| 235 | } |
| 236 | |
| 237 | # Apply the component override and default to universe. |
| 238 | @@ -814,7 +835,6 @@ |
| 239 | # |
| 240 | # Actually processing accepted or rejected uploads -- and mailing people |
| 241 | # |
| 242 | - |
| 243 | def do_accept(self, notify=True): |
| 244 | """Accept the upload into the queue. |
| 245 | |
| 246 | @@ -921,7 +941,6 @@ |
| 247 | # |
| 248 | # Inserting stuff in the database |
| 249 | # |
| 250 | - |
| 251 | def storeObjectsInDatabase(self): |
| 252 | """Insert this nascent upload into the database.""" |
| 253 | |
| 254 | @@ -1044,8 +1063,7 @@ |
| 255 | # See if there is an archive to override with. |
| 256 | distribution = self.policy.distroseries.distribution |
| 257 | archive = distribution.getArchiveByComponent( |
| 258 | - PARTNER_COMPONENT_NAME |
| 259 | - ) |
| 260 | + PARTNER_COMPONENT_NAME) |
| 261 | |
| 262 | # Check for data problems: |
| 263 | if not archive: |
| 264 | |
| 265 | === modified file 'lib/lp/archiveuploader/tests/nascentupload-announcements.txt' |
| 266 | --- lib/lp/archiveuploader/tests/nascentupload-announcements.txt 2010-07-24 09:12:37 +0000 |
| 267 | +++ lib/lp/archiveuploader/tests/nascentupload-announcements.txt 2010-08-04 22:12:18 +0000 |
| 268 | @@ -79,7 +79,7 @@ |
| 269 | >>> sync_policy = getPolicy( |
| 270 | ... name='sync', distro='ubuntu', distroseries='hoary') |
| 271 | |
| 272 | - >>> bar_src = NascentUpload( |
| 273 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 274 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 275 | ... sync_policy, mock_logger_quiet) |
| 276 | >>> bar_src.process() |
| 277 | @@ -132,7 +132,7 @@ |
| 278 | |
| 279 | Uploading the same package again will result in a rejection email: |
| 280 | |
| 281 | - >>> bar_src = NascentUpload( |
| 282 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 283 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 284 | ... sync_policy, mock_logger_quiet) |
| 285 | >>> bar_src.process() |
| 286 | @@ -270,7 +270,7 @@ |
| 287 | >>> ppa_policy.archive = name16_ppa |
| 288 | >>> ppa_policy.setDistroSeriesAndPocket('hoary') |
| 289 | |
| 290 | - >>> ppa_bar_src = NascentUpload( |
| 291 | + >>> ppa_bar_src = NascentUpload.from_changesfile_path( |
| 292 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 293 | ... ppa_policy, mock_logger_quiet) |
| 294 | >>> ppa_bar_src.process() |
| 295 | @@ -308,7 +308,7 @@ |
| 296 | ... name='sync', distro='ubuntu', distroseries='hoary') |
| 297 | >>> modified_sync_policy.can_upload_binaries = True |
| 298 | |
| 299 | - >>> bar_src = NascentUpload( |
| 300 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 301 | ... datadir('suite/bar_1.0-1_binary/bar_1.0-1_i386.changes'), |
| 302 | ... modified_sync_policy, mock_logger_quiet) |
| 303 | >>> bar_src.process() |
| 304 | @@ -336,7 +336,7 @@ |
| 305 | >>> modified_sync_policy = getPolicy( |
| 306 | ... name='sync', distro='ubuntu', distroseries='hoary') |
| 307 | |
| 308 | - >>> lang_pack = NascentUpload( |
| 309 | + >>> lang_pack = NascentUpload.from_changesfile_path( |
| 310 | ... datadir('language-packs/language-pack-pt_1.0-1_source.changes'), |
| 311 | ... modified_sync_policy, mock_logger_quiet) |
| 312 | >>> lang_pack.process() |
| 313 | @@ -367,7 +367,7 @@ |
| 314 | >>> modified_sync_policy = getPolicy( |
| 315 | ... name='sync', distro='ubuntu', distroseries='hoary') |
| 316 | |
| 317 | - >>> lang_pack = NascentUpload( |
| 318 | + >>> lang_pack = NascentUpload.from_changesfile_path( |
| 319 | ... datadir('language-packs/language-pack-pt_1.0-2_source.changes'), |
| 320 | ... modified_sync_policy, mock_logger_quiet) |
| 321 | >>> lang_pack.process() |
| 322 | @@ -398,7 +398,7 @@ |
| 323 | ... name='insecure', distro='ubuntu', distroseries=None) |
| 324 | >>> insecure_policy.setDistroSeriesAndPocket('hoary-updates') |
| 325 | |
| 326 | - >>> lang_pack = NascentUpload( |
| 327 | + >>> lang_pack = NascentUpload.from_changesfile_path( |
| 328 | ... datadir('language-packs/language-pack-pt_1.0-3_source.changes'), |
| 329 | ... insecure_policy, mock_logger_quiet) |
| 330 | >>> lang_pack.process() |
| 331 | @@ -423,7 +423,7 @@ |
| 332 | An UNAPPROVED binary upload via insecure will send one email saying that |
| 333 | the upload is waiting for approval: |
| 334 | |
| 335 | - >>> bar_src = NascentUpload( |
| 336 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 337 | ... datadir('suite/bar_1.0-2/bar_1.0-2_source.changes'), |
| 338 | ... insecure_policy, mock_logger_quiet) |
| 339 | >>> bar_src.process() |
| 340 | @@ -451,7 +451,7 @@ |
| 341 | >>> unapproved_backports_policy = getPolicy( |
| 342 | ... name='insecure', distro='ubuntu', distroseries=None) |
| 343 | >>> unapproved_backports_policy.setDistroSeriesAndPocket('hoary-backports') |
| 344 | - >>> bar_src = NascentUpload( |
| 345 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 346 | ... datadir('suite/bar_1.0-3_valid/bar_1.0-3_source.changes'), |
| 347 | ... unapproved_backports_policy, mock_logger_quiet) |
| 348 | >>> bar_src.process() |
| 349 | @@ -479,7 +479,7 @@ |
| 350 | ... name='sync', distro='ubuntu', distroseries=None) |
| 351 | >>> modified_sync_policy.setDistroSeriesAndPocket('hoary-backports') |
| 352 | |
| 353 | - >>> bar_src = NascentUpload( |
| 354 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 355 | ... datadir('suite/bar_1.0-4/bar_1.0-4_source.changes'), |
| 356 | ... modified_sync_policy, mock_logger_quiet) |
| 357 | >>> bar_src.process() |
| 358 | @@ -536,7 +536,7 @@ |
| 359 | ... name='security', distro='ubuntu', distroseries=None) |
| 360 | >>> security_policy.setDistroSeriesAndPocket('hoary-security') |
| 361 | |
| 362 | - >>> bar_src = NascentUpload( |
| 363 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 364 | ... datadir('suite/bar_1.0-2/bar_1.0-2_source.changes'), |
| 365 | ... security_policy, mock_logger_quiet) |
| 366 | >>> bar_src.process() |
| 367 | @@ -729,7 +729,7 @@ |
| 368 | |
| 369 | >>> hoary.status = SeriesStatus.DEVELOPMENT |
| 370 | |
| 371 | - >>> bar_src = NascentUpload( |
| 372 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 373 | ... datadir( |
| 374 | ... 'suite/bar_1.0-5_debian_auto_sync/bar_1.0-5_source.changes'), |
| 375 | ... sync_policy, mock_logger_quiet) |
| 376 | @@ -749,7 +749,7 @@ |
| 377 | |
| 378 | In contrast, manual sync uploads do generate the announcement: |
| 379 | |
| 380 | - >>> bar_src = NascentUpload( |
| 381 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 382 | ... datadir( |
| 383 | ... 'suite/bar_1.0-6/bar_1.0-6_source.changes'), |
| 384 | ... sync_policy, mock_logger_quiet) |
| 385 | @@ -789,7 +789,7 @@ |
| 386 | ... name='security', distro='ubuntu', distroseries=None) |
| 387 | >>> security_policy.setDistroSeriesAndPocket('hoary-security') |
| 388 | |
| 389 | - >>> bar_bin = NascentUpload( |
| 390 | + >>> bar_bin = NascentUpload.from_changesfile_path( |
| 391 | ... datadir('suite/bar_1.0-2_binary/bar_1.0-2_i386.changes'), |
| 392 | ... security_policy, mock_logger_quiet) |
| 393 | >>> bar_bin.process() |
| 394 | @@ -849,7 +849,7 @@ |
| 395 | >>> modified_sync_policy.can_upload_mixed = True |
| 396 | >>> modified_sync_policy.can_upload_binaries = True |
| 397 | >>> modified_sync_policy.can_upload_source = True |
| 398 | - >>> foo_v1 = NascentUpload( |
| 399 | + >>> foo_v1 = NascentUpload.from_changesfile_path( |
| 400 | ... datadir('suite/foo_1.0-1_mixed/foo_1.0-1_i386.changes'), |
| 401 | ... modified_sync_policy, mock_logger_quiet) |
| 402 | >>> foo_v1.process() |
| 403 | @@ -867,7 +867,7 @@ |
| 404 | >>> security_policy = getPolicy( |
| 405 | ... name='security', distro='ubuntu', distroseries=None) |
| 406 | >>> security_policy.setDistroSeriesAndPocket('hoary-security') |
| 407 | - >>> foo_mixed = NascentUpload( |
| 408 | + >>> foo_mixed = NascentUpload.from_changesfile_path( |
| 409 | ... datadir('suite/foo_1.0-2.1/foo_1.0-2.1_source.changes'), |
| 410 | ... security_policy, mock_logger_quiet) |
| 411 | >>> foo_mixed.process() |
| 412 | @@ -905,7 +905,7 @@ |
| 413 | >>> sync_policy = getPolicy( |
| 414 | ... name='sync', distro='ubuntu', distroseries='hoary') |
| 415 | |
| 416 | - >>> bar_src = NascentUpload( |
| 417 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 418 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 419 | ... sync_policy, mock_logger_quiet) |
| 420 | >>> bar_src.process() |
| 421 | @@ -975,7 +975,7 @@ |
| 422 | >>> hoary.status = SeriesStatus.DEVELOPMENT |
| 423 | >>> anything_policy = getPolicy( |
| 424 | ... name='anything', distro='ubuntu', distroseries='hoary') |
| 425 | - >>> bar_upload = NascentUpload( |
| 426 | + >>> bar_upload = NascentUpload.from_changesfile_path( |
| 427 | ... datadir( |
| 428 | ... 'suite/bar_1.0-10_utf8_changesfile/bar_1.0-10_source.changes'), |
| 429 | ... anything_policy, mock_logger_quiet) |
| 430 | @@ -1106,7 +1106,8 @@ |
| 431 | |
| 432 | And then try to upload using the changes file with the malformed name. |
| 433 | |
| 434 | - >>> bar_src = NascentUpload(copyp, sync_policy, mock_logger_quiet) |
| 435 | + >>> bar_src = NascentUpload.from_changesfile_path( |
| 436 | + ... copyp, sync_policy, mock_logger_quiet) |
| 437 | >>> bar_src.process() |
| 438 | Traceback (most recent call last): |
| 439 | ... |
| 440 | |
| 441 | === modified file 'lib/lp/archiveuploader/tests/nascentupload-packageset.txt' |
| 442 | --- lib/lp/archiveuploader/tests/nascentupload-packageset.txt 2010-05-11 14:09:44 +0000 |
| 443 | +++ lib/lp/archiveuploader/tests/nascentupload-packageset.txt 2010-08-04 22:12:18 +0000 |
| 444 | @@ -31,7 +31,7 @@ |
| 445 | This time the upload will fail because the ACLs don't let |
| 446 | "name16", the key owner, upload a package. |
| 447 | |
| 448 | - >>> bar_failed = NascentUpload( |
| 449 | + >>> bar_failed = NascentUpload.from_changesfile_path( |
| 450 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 451 | ... insecure_policy, mock_logger_quiet) |
| 452 | |
| 453 | @@ -111,7 +111,7 @@ |
| 454 | |
| 455 | Let's retry the upload. |
| 456 | |
| 457 | - >>> bar_failed = NascentUpload( |
| 458 | + >>> bar_failed = NascentUpload.from_changesfile_path( |
| 459 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 460 | ... insecure_policy, mock_logger_quiet) |
| 461 | |
| 462 | @@ -166,7 +166,7 @@ |
| 463 | |
| 464 | >>> commit() |
| 465 | >>> LaunchpadZopelessLayer.switchDbUser('uploader') |
| 466 | - >>> bar2 = NascentUpload( |
| 467 | + >>> bar2 = NascentUpload.from_changesfile_path( |
| 468 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 469 | ... insecure_policy, mock_logger_quiet) |
| 470 | >>> bar2.process() |
| 471 | |
| 472 | === modified file 'lib/lp/archiveuploader/tests/nascentupload-security-uploads.txt' |
| 473 | --- lib/lp/archiveuploader/tests/nascentupload-security-uploads.txt 2010-07-24 09:12:37 +0000 |
| 474 | +++ lib/lp/archiveuploader/tests/nascentupload-security-uploads.txt 2010-08-04 22:12:18 +0000 |
| 475 | @@ -55,7 +55,7 @@ |
| 476 | The upload in question contains a source and its 2 builds for i386 and |
| 477 | powerpc: |
| 478 | |
| 479 | - >>> foo_mixed_upload = NascentUpload( |
| 480 | + >>> foo_mixed_upload = NascentUpload.from_changesfile_path( |
| 481 | ... datadir('suite/foo_1.0-1_multi_binary/foo_1.0-1_multi.changes'), |
| 482 | ... security_policy, mock_logger_quiet) |
| 483 | >>> foo_mixed_upload.process() |
| 484 | @@ -121,7 +121,7 @@ |
| 485 | security upload, for example, detecting that the source and the |
| 486 | binaries sent do not match. |
| 487 | |
| 488 | - >>> bar_mixed_upload = NascentUpload( |
| 489 | + >>> bar_mixed_upload = NascentUpload.from_changesfile_path( |
| 490 | ... datadir('suite/foo_1.0-1_broken_binary/bar_1.0-1_multi.changes'), |
| 491 | ... security_policy, mock_logger_quiet) |
| 492 | >>> bar_mixed_upload.process() |
| 493 | |
| 494 | === modified file 'lib/lp/archiveuploader/tests/nascentupload.txt' |
| 495 | --- lib/lp/archiveuploader/tests/nascentupload.txt 2010-07-24 09:12:37 +0000 |
| 496 | +++ lib/lp/archiveuploader/tests/nascentupload.txt 2010-08-04 22:12:18 +0000 |
| 497 | @@ -45,14 +45,15 @@ |
| 498 | doc/nascentuploadfile.txt) object based on that. If anything goes |
| 499 | wrong during this process FatalUploadError is raised: |
| 500 | |
| 501 | - >>> NascentUpload(datadir("DOES-NOT-EXIST"), buildd_policy, mock_logger) |
| 502 | + >>> NascentUpload.from_changesfile_path( |
| 503 | + ... datadir("DOES-NOT-EXIST"), buildd_policy, mock_logger) |
| 504 | Traceback (most recent call last): |
| 505 | ... |
| 506 | FatalUploadError:... |
| 507 | |
| 508 | Otherwise a ChangesFile object is ready to use. |
| 509 | |
| 510 | - >>> quodlibet = NascentUpload( |
| 511 | + >>> quodlibet = NascentUpload.from_changesfile_path( |
| 512 | ... datadir("quodlibet_0.13.1-1_i386.changes"), |
| 513 | ... anything_policy, mock_logger_quiet) |
| 514 | |
| 515 | @@ -110,7 +111,7 @@ |
| 516 | is 'native' (only a TARBALL, no diff + orig) or 'has_orig' (uses ORIG |
| 517 | + DIFF source form). |
| 518 | |
| 519 | - >>> ed_source_upload = NascentUpload( |
| 520 | + >>> ed_source_upload = NascentUpload.from_changesfile_path( |
| 521 | ... datadir("ed_0.2-20_i386.changes.source-only-unsigned"), |
| 522 | ... sync_policy, mock_logger_quiet) |
| 523 | |
| 524 | @@ -167,7 +168,7 @@ |
| 525 | |
| 526 | Let's try a simple binary upload: |
| 527 | |
| 528 | - >>> ed_binary_upload = NascentUpload( |
| 529 | + >>> ed_binary_upload = NascentUpload.from_changesfile_path( |
| 530 | ... datadir("ed_0.2-20_i386.changes.binary-only-unsigned"), |
| 531 | ... buildd_policy, mock_logger_quiet) |
| 532 | |
| 533 | @@ -220,7 +221,7 @@ |
| 534 | >>> modified_buildd_policy.can_upload_source = True |
| 535 | >>> modified_buildd_policy.can_upload_mixed = True |
| 536 | |
| 537 | - >>> ed_mismatched_upload = NascentUpload( |
| 538 | + >>> ed_mismatched_upload = NascentUpload.from_changesfile_path( |
| 539 | ... datadir("ed_0.2-20_i386.changes.mismatched-arch-unsigned"), |
| 540 | ... modified_buildd_policy, mock_logger_quiet) |
| 541 | |
| 542 | @@ -258,7 +259,7 @@ |
| 543 | >>> modified_insecure_policy.can_upload_binaries = True |
| 544 | >>> modified_insecure_policy.can_upload_mixed = True |
| 545 | |
| 546 | - >>> ed_mixed_upload = NascentUpload( |
| 547 | + >>> ed_mixed_upload = NascentUpload.from_changesfile_path( |
| 548 | ... datadir("ed_0.2-20_i386.changes"), |
| 549 | ... modified_insecure_policy, mock_logger_quiet) |
| 550 | |
| 551 | @@ -317,7 +318,7 @@ |
| 552 | |
| 553 | First up, construct an upload of just the ed source... |
| 554 | |
| 555 | - >>> ed_src = NascentUpload( |
| 556 | + >>> ed_src = NascentUpload.from_changesfile_path( |
| 557 | ... datadir('split-upload-test/ed_0.2-20_source.changes'), |
| 558 | ... sync_policy, mock_logger_quiet) |
| 559 | >>> ed_src.process() |
| 560 | @@ -407,7 +408,7 @@ |
| 561 | maintainer's side), however it cannot store a proper |
| 562 | SourcePackageRelease.copyright content. See bug #134567. |
| 563 | |
| 564 | - >>> nocopyright_src = NascentUpload( |
| 565 | + >>> nocopyright_src = NascentUpload.from_changesfile_path( |
| 566 | ... datadir('suite/nocopyright_1.0-1/nocopyright_1.0-1_source.changes'), |
| 567 | ... sync_policy, mock_logger_quiet) |
| 568 | >>> nocopyright_src.process() |
| 569 | @@ -451,7 +452,7 @@ |
| 570 | published in the archive, which provides the same source package name |
| 571 | and source package version for the distroseries in question. |
| 572 | |
| 573 | - >>> ed_src_dup = NascentUpload( |
| 574 | + >>> ed_src_dup = NascentUpload.from_changesfile_path( |
| 575 | ... datadir('split-upload-test/ed_0.2-20_source.changes'), |
| 576 | ... sync_policy, mock_logger_quiet) |
| 577 | >>> ed_src_dup.process() |
| 578 | @@ -519,7 +520,7 @@ |
| 579 | ... name='sync', distro='ubuntu', distroseries='hoary') |
| 580 | >>> modified_sync_policy.can_upload_binaries = True |
| 581 | |
| 582 | - >>> ed_bin = NascentUpload( |
| 583 | + >>> ed_bin = NascentUpload.from_changesfile_path( |
| 584 | ... datadir('split-upload-test/ed_0.2-20_i386.changes'), |
| 585 | ... modified_sync_policy, mock_logger_quiet) |
| 586 | |
| 587 | @@ -606,7 +607,7 @@ |
| 588 | |
| 589 | Upload new source 'multibar', step 1: |
| 590 | |
| 591 | - >>> multibar_src_upload = NascentUpload( |
| 592 | + >>> multibar_src_upload = NascentUpload.from_changesfile_path( |
| 593 | ... datadir('suite/multibar_1.0-1/multibar_1.0-1_source.changes'), |
| 594 | ... sync_policy, mock_logger_quiet) |
| 595 | >>> multibar_src_upload.process() |
| 596 | @@ -658,7 +659,7 @@ |
| 597 | ... name='buildd', distro='ubuntu', distroseries='hoary', |
| 598 | ... buildid=multibar_build.id) |
| 599 | |
| 600 | - >>> multibar_bin_upload = NascentUpload( |
| 601 | + >>> multibar_bin_upload = NascentUpload.from_changesfile_path( |
| 602 | ... datadir('suite/multibar_1.0-1/multibar_1.0-1_i386.changes'), |
| 603 | ... buildd_policy, mock_logger_quiet) |
| 604 | >>> multibar_bin_upload.process() |
| 605 | @@ -727,7 +728,7 @@ |
| 606 | >>> norelease_sync_policy = getPolicy( |
| 607 | ... name='sync', distro='ubuntu') |
| 608 | |
| 609 | - >>> ed_src = NascentUpload( |
| 610 | + >>> ed_src = NascentUpload.from_changesfile_path( |
| 611 | ... datadir('updates-upload-test/ed_0.2-20_source.changes'), |
| 612 | ... norelease_sync_policy, mock_logger_quiet) |
| 613 | >>> ed_src.process() |
| 614 | @@ -756,7 +757,7 @@ |
| 615 | Check the uploader behaviour against a missing orig.tar.gz file, |
| 616 | bug # 30741. |
| 617 | |
| 618 | - >>> ed21_src = NascentUpload( |
| 619 | + >>> ed21_src = NascentUpload.from_changesfile_path( |
| 620 | ... datadir('ed-0.2-21/ed_0.2-21_source.changes'), |
| 621 | ... sync_policy, mock_logger_quiet) |
| 622 | >>> ed21_src.process() |
| 623 | @@ -774,7 +775,7 @@ |
| 624 | 'Standards-Version' field in DSC. See bug #75874 for further |
| 625 | information. |
| 626 | |
| 627 | - >>> inst_src = NascentUpload( |
| 628 | + >>> inst_src = NascentUpload.from_changesfile_path( |
| 629 | ... datadir('test75874_0.1_source.changes'), |
| 630 | ... sync_policy, mock_logger_quiet) |
| 631 | >>> inst_src.process() |
| 632 | @@ -822,7 +823,7 @@ |
| 633 | When using 'insecure' policy, NascentUpload instace stores the DSC |
| 634 | sigining key reference as an IGPGKey: |
| 635 | |
| 636 | - >>> bar_ok = NascentUpload( |
| 637 | + >>> bar_ok = NascentUpload.from_changesfile_path( |
| 638 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 639 | ... insecure_policy, mock_logger_quiet) |
| 640 | >>> bar_ok.process() |
| 641 | @@ -871,7 +872,7 @@ |
| 642 | This time the upload will fail because the ACLs don't let |
| 643 | "name16", the key owner, upload a package. |
| 644 | |
| 645 | - >>> bar_failed = NascentUpload( |
| 646 | + >>> bar_failed = NascentUpload.from_changesfile_path( |
| 647 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 648 | ... insecure_policy, mock_logger_quiet) |
| 649 | |
| 650 | @@ -917,7 +918,7 @@ |
| 651 | |
| 652 | Now try the "bar" upload: |
| 653 | |
| 654 | - >>> bar2 = NascentUpload( |
| 655 | + >>> bar2 = NascentUpload.from_changesfile_path( |
| 656 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 657 | ... insecure_policy, mock_logger_quiet) |
| 658 | >>> bar2.process() |
| 659 | @@ -943,7 +944,7 @@ |
| 660 | Make this upload policy pertain to the copy archive. |
| 661 | |
| 662 | >>> copy_archive_policy.archive = copy_archive |
| 663 | - >>> quodlibet = NascentUpload( |
| 664 | + >>> quodlibet = NascentUpload.from_changesfile_path( |
| 665 | ... datadir("quodlibet_0.13.1-1_i386.changes"), |
| 666 | ... copy_archive_policy, mock_logger_quiet) |
| 667 | |
| 668 | |
| 669 | === added file 'lib/lp/archiveuploader/tests/test_nascentupload.py' |
| 670 | --- lib/lp/archiveuploader/tests/test_nascentupload.py 1970-01-01 00:00:00 +0000 |
| 671 | +++ lib/lp/archiveuploader/tests/test_nascentupload.py 2010-08-04 22:12:18 +0000 |
| 672 | @@ -0,0 +1,85 @@ |
| 673 | +# Copyright 2010 Canonical Ltd. This software is licensed under the |
| 674 | +# GNU Affero General Public License version 3 (see the file LICENSE). |
| 675 | + |
| 676 | +"""Test NascentUpload functionality.""" |
| 677 | + |
| 678 | +__metaclass__ = type |
| 679 | + |
| 680 | +from testtools import TestCase |
| 681 | + |
| 682 | +from canonical.testing import LaunchpadZopelessLayer |
| 683 | +from lp.archiveuploader.changesfile import determine_file_class_and_name |
| 684 | +from lp.archiveuploader.nascentupload import NascentUpload |
| 685 | +from lp.archiveuploader.tests import mock_logger |
| 686 | + |
| 687 | + |
| 688 | +class FakeChangesFile: |
| 689 | + |
| 690 | + def __init__(self): |
| 691 | + self.files = [] |
| 692 | + |
| 693 | + |
| 694 | +class TestMatchDDEBs(TestCase): |
| 695 | + """Tests that NascentUpload correctly links DEBs to their DDEBs. |
| 696 | + |
| 697 | + Also verifies detection of DDEB-related error cases. |
| 698 | + """ |
| 699 | + |
| 700 | + layer = LaunchpadZopelessLayer |
| 701 | + |
| 702 | + def setUp(self): |
| 703 | + super(TestMatchDDEBs, self).setUp() |
| 704 | + self.changes = FakeChangesFile() |
| 705 | + self.upload = NascentUpload(self.changes, None, mock_logger) |
| 706 | + |
| 707 | + def addFile(self, filename): |
| 708 | + """Add a file of the right type to the upload.""" |
| 709 | + package, cls = determine_file_class_and_name(filename) |
| 710 | + file = cls( |
| 711 | + filename, None, 100, 'devel', 'extra', package, '666', |
| 712 | + self.changes, None, self.upload.logger) |
| 713 | + self.changes.files.append(file) |
| 714 | + |
| 715 | + def assertMatchDDEBErrors(self, error_list): |
| 716 | + self.assertEquals( |
| 717 | + error_list, [str(e) for e in self.upload._matchDDEBs()]) |
| 718 | + |
| 719 | + def testNoLinksWithNoBinaries(self): |
| 720 | + # No links will be made if there are no binaries whatsoever. |
| 721 | + self.addFile('something_1.0.diff.gz') |
| 722 | + self.assertMatchDDEBErrors([]) |
| 723 | + |
| 724 | + def testNoLinksWithJustDEBs(self): |
| 725 | + # No links will be made if there are no DDEBs. |
| 726 | + self.addFile('blah_1.0_all.deb') |
| 727 | + self.addFile('libblah_1.0_i386.deb') |
| 728 | + self.assertMatchDDEBErrors([]) |
| 729 | + for file in self.changes.files: |
| 730 | + self.assertIs(None, file.ddeb_file) |
| 731 | + |
| 732 | + def testLinksMatchingDDEBs(self): |
| 733 | + # DDEBs will be linked to their matching DEBs. |
| 734 | + self.addFile('blah_1.0_all.deb') |
| 735 | + self.addFile('libblah_1.0_i386.deb') |
| 736 | + self.addFile('libblah-dbgsym_1.0_i386.ddeb') |
| 737 | + self.assertMatchDDEBErrors([]) |
| 738 | + self.assertIs(None, self.changes.files[0].ddeb_file) |
| 739 | + self.assertIs(self.changes.files[2], self.changes.files[1].ddeb_file) |
| 740 | + self.assertIs(self.changes.files[1], self.changes.files[2].deb_file) |
| 741 | + self.assertIs(None, self.changes.files[2].ddeb_file) |
| 742 | + |
| 743 | + def testDuplicateDDEBsCauseErrors(self): |
| 744 | + # An error will be raised if a DEB has more than one matching |
| 745 | + # DDEB. |
| 746 | + self.addFile('libblah_1.0_i386.deb') |
| 747 | + self.addFile('libblah-dbgsym_1.0_i386.ddeb') |
| 748 | + self.addFile('libblah-dbgsym_1.0_i386.ddeb') |
| 749 | + self.assertMatchDDEBErrors( |
| 750 | + ['Duplicated debug packages: libblah-dbgsym 666 (i386)']) |
| 751 | + |
| 752 | + def testMismatchedDDEBsCauseErrors(self): |
| 753 | + # An error will be raised if a DDEB has no matching DEB. |
| 754 | + self.addFile('libblah_1.0_i386.deb') |
| 755 | + self.addFile('libblah-dbgsym_1.0_amd64.ddeb') |
| 756 | + self.assertMatchDDEBErrors( |
| 757 | + ['Orphaned debug packages: libblah-dbgsym 666 (amd64)']) |
| 758 | |
| 759 | === modified file 'lib/lp/archiveuploader/tests/test_nascentupload_documentation.py' |
| 760 | --- lib/lp/archiveuploader/tests/test_nascentupload_documentation.py 2009-06-24 23:33:29 +0000 |
| 761 | +++ lib/lp/archiveuploader/tests/test_nascentupload_documentation.py 2010-08-04 22:12:18 +0000 |
| 762 | @@ -26,20 +26,25 @@ |
| 763 | def getUploadForSource(upload_path): |
| 764 | """Return a NascentUpload object for a source.""" |
| 765 | policy = getPolicy(name='sync', distro='ubuntu', distroseries='hoary') |
| 766 | - return NascentUpload(datadir(upload_path), policy, mock_logger_quiet) |
| 767 | + return NascentUpload.from_changesfile_path( |
| 768 | + datadir(upload_path), policy, mock_logger_quiet) |
| 769 | + |
| 770 | |
| 771 | def getPPAUploadForSource(upload_path, ppa): |
| 772 | """Return a NascentUpload object for a PPA source.""" |
| 773 | policy = getPolicy(name='insecure', distro='ubuntu', distroseries='hoary') |
| 774 | policy.archive = ppa |
| 775 | - return NascentUpload(datadir(upload_path), policy, mock_logger_quiet) |
| 776 | + return NascentUpload.from_changesfile_path( |
| 777 | + datadir(upload_path), policy, mock_logger_quiet) |
| 778 | + |
| 779 | |
| 780 | def getUploadForBinary(upload_path): |
| 781 | """Return a NascentUpload object for binaries.""" |
| 782 | policy = getPolicy(name='sync', distro='ubuntu', distroseries='hoary') |
| 783 | policy.can_upload_binaries = True |
| 784 | policy.can_upload_mixed = True |
| 785 | - return NascentUpload(datadir(upload_path), policy, mock_logger_quiet) |
| 786 | + return NascentUpload.from_changesfile_path( |
| 787 | + datadir(upload_path), policy, mock_logger_quiet) |
| 788 | |
| 789 | |
| 790 | def testGlobalsSetup(test): |
| 791 | |
| 792 | === modified file 'lib/lp/archiveuploader/uploadprocessor.py' |
| 793 | --- lib/lp/archiveuploader/uploadprocessor.py 2010-08-02 09:40:22 +0000 |
| 794 | +++ lib/lp/archiveuploader/uploadprocessor.py 2010-08-04 22:12:18 +0000 |
| 795 | @@ -335,7 +335,8 @@ |
| 796 | # The path we want for NascentUpload is the path to the folder |
| 797 | # containing the changes file (and the other files referenced by it). |
| 798 | changesfile_path = os.path.join(upload_path, changes_file) |
| 799 | - upload = NascentUpload(changesfile_path, policy, self.log) |
| 800 | + upload = NascentUpload.from_changesfile_path( |
| 801 | + changesfile_path, policy, self.log) |
| 802 | |
| 803 | # Reject source upload to buildd upload paths. |
| 804 | first_path = relative_path.split(os.path.sep)[0] |
| 805 | |
| 806 | === modified file 'lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt' |
| 807 | --- lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt 2009-06-12 16:36:02 +0000 |
| 808 | +++ lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt 2010-08-04 22:12:18 +0000 |
| 809 | @@ -260,7 +260,7 @@ |
| 810 | ... distroseries='hoary') |
| 811 | >>> sync_policy.can_upload_binaries = True |
| 812 | >>> sync_policy.can_upload_mixed = True |
| 813 | - >>> foo_upload = NascentUpload( |
| 814 | + >>> foo_upload = NascentUpload.from_changesfile_path( |
| 815 | ... datadir('suite/foo_1.0-2_multi_binary/foo_1.0-2_i386.changes'), |
| 816 | ... sync_policy, mock_logger_quiet) |
| 817 | >>> foo_upload.process() |
| 818 | |
| 819 | === modified file 'lib/lp/soyuz/doc/build-failedtoupload-workflow.txt' |
| 820 | --- lib/lp/soyuz/doc/build-failedtoupload-workflow.txt 2010-05-12 08:17:20 +0000 |
| 821 | +++ lib/lp/soyuz/doc/build-failedtoupload-workflow.txt 2010-08-04 22:12:18 +0000 |
| 822 | @@ -165,7 +165,7 @@ |
| 823 | ... distroseries=failedtoupload_candidate.distro_series.name, |
| 824 | ... buildid=failedtoupload_candidate.id) |
| 825 | |
| 826 | - >>> cdrkit_bin_upload = NascentUpload( |
| 827 | + >>> cdrkit_bin_upload = NascentUpload.from_changesfile_path( |
| 828 | ... datadir('suite/cdrkit_1.0/cdrkit_1.0_i386.changes'), |
| 829 | ... buildd_policy, mock_logger_quiet) |
| 830 | >>> cdrkit_bin_upload.process() |
| 831 | |
| 832 | === modified file 'lib/lp/soyuz/doc/distroseriesqueue-ddtp-tarball.txt' |
| 833 | --- lib/lp/soyuz/doc/distroseriesqueue-ddtp-tarball.txt 2009-05-13 14:05:27 +0000 |
| 834 | +++ lib/lp/soyuz/doc/distroseriesqueue-ddtp-tarball.txt 2010-08-04 22:12:18 +0000 |
| 835 | @@ -47,7 +47,7 @@ |
| 836 | >>> sync_policy = getPolicy( |
| 837 | ... name='sync', distro='ubuntutest', distroseries=None) |
| 838 | |
| 839 | - >>> upload = NascentUpload( |
| 840 | + >>> upload = NascentUpload.from_changesfile_path( |
| 841 | ... datadir('ddtp-tarball/translations-main_20060728.changes'), |
| 842 | ... sync_policy, mock_logger_quiet) |
| 843 | >>> upload.process() |
| 844 | @@ -60,7 +60,7 @@ |
| 845 | >>> insecure_policy = getPolicy( |
| 846 | ... name='insecure', distro='ubuntutest', distroseries=None) |
| 847 | |
| 848 | - >>> upload = NascentUpload( |
| 849 | + >>> upload = NascentUpload.from_changesfile_path( |
| 850 | ... datadir('ddtp-tarball/translations-main_20060728_all.changes'), |
| 851 | ... insecure_policy, mock_logger) |
| 852 | DEBUG: Verifying signature on translations-main_20060728_all.changes |
| 853 | @@ -228,7 +228,7 @@ |
| 854 | >>> insecure_policy = getPolicy( |
| 855 | ... name='insecure', distro='ubuntutest', distroseries=None) |
| 856 | |
| 857 | - >>> upload = NascentUpload( |
| 858 | + >>> upload = NascentUpload.from_changesfile_path( |
| 859 | ... datadir('ddtp-tarball/translations-main_20060817_all.changes'), |
| 860 | ... insecure_policy, mock_logger_quiet) |
| 861 | >>> upload.process() |
| 862 | |
| 863 | === modified file 'lib/lp/soyuz/doc/distroseriesqueue-debian-installer.txt' |
| 864 | --- lib/lp/soyuz/doc/distroseriesqueue-debian-installer.txt 2009-05-13 14:05:27 +0000 |
| 865 | +++ lib/lp/soyuz/doc/distroseriesqueue-debian-installer.txt 2010-08-04 22:12:18 +0000 |
| 866 | @@ -57,7 +57,7 @@ |
| 867 | |
| 868 | >>> anything_policy.distroseries.changeslist = 'announce@example.com' |
| 869 | |
| 870 | - >>> upload = NascentUpload( |
| 871 | + >>> upload = NascentUpload.from_changesfile_path( |
| 872 | ... datadir( |
| 873 | ... 'debian-installer/debian-installer_20070214ubuntu1_i386.changes'), |
| 874 | ... anything_policy, mock_logger_quiet) |
| 875 | |
| 876 | === modified file 'lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt' |
| 877 | --- lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt 2010-08-03 22:03:56 +0000 |
| 878 | +++ lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt 2010-08-04 22:12:18 +0000 |
| 879 | @@ -23,7 +23,7 @@ |
| 880 | >>> sync_policy = getPolicy( |
| 881 | ... name='sync', distro='ubuntutest', distroseries=None) |
| 882 | |
| 883 | - >>> upload = NascentUpload( |
| 884 | + >>> upload = NascentUpload.from_changesfile_path( |
| 885 | ... datadir('dist-upgrader/dist-upgrader_20060302.0120.changes'), |
| 886 | ... sync_policy, mock_logger) |
| 887 | DEBUG: Changes file can be unsigned. |
| 888 | @@ -37,7 +37,7 @@ |
| 889 | >>> insecure_policy = getPolicy( |
| 890 | ... name='insecure', distro='ubuntutest', distroseries=None) |
| 891 | |
| 892 | - >>> upload = NascentUpload( |
| 893 | + >>> upload = NascentUpload.from_changesfile_path( |
| 894 | ... datadir('dist-upgrader/dist-upgrader_20060302.0120_all.changes'), |
| 895 | ... insecure_policy, mock_logger) |
| 896 | DEBUG: Verifying signature on dist-upgrader_20060302.0120_all.changes |
| 897 | @@ -214,7 +214,7 @@ |
| 898 | >>> insecure_policy = getPolicy( |
| 899 | ... name='insecure', distro='ubuntutest', distroseries=None) |
| 900 | |
| 901 | - >>> upload = NascentUpload( |
| 902 | + >>> upload = NascentUpload.from_changesfile_path( |
| 903 | ... datadir('dist-upgrader/dist-upgrader_20070219.1234_all.changes'), |
| 904 | ... insecure_policy, mock_logger) |
| 905 | DEBUG: Verifying signature on dist-upgrader_20070219.1234_all.changes |
| 906 | @@ -287,7 +287,7 @@ |
| 907 | |
| 908 | >>> insecure_policy.archive = foobar_archive |
| 909 | |
| 910 | - >>> ppa_upload = NascentUpload( |
| 911 | + >>> ppa_upload = NascentUpload.from_changesfile_path( |
| 912 | ... datadir('dist-upgrader/dist-upgrader_20060302.0120_all.changes'), |
| 913 | ... insecure_policy, mock_logger) |
| 914 | DEBUG: Verifying signature on dist-upgrader_20060302.0120_all.changes |
| 915 | |
| 916 | === modified file 'lib/lp/soyuz/doc/distroseriesqueue-translations.txt' |
| 917 | --- lib/lp/soyuz/doc/distroseriesqueue-translations.txt 2010-05-14 04:47:38 +0000 |
| 918 | +++ lib/lp/soyuz/doc/distroseriesqueue-translations.txt 2010-08-04 22:12:18 +0000 |
| 919 | @@ -77,7 +77,7 @@ |
| 920 | ... name='buildd', distro='ubuntu', distroseries='dapper', |
| 921 | ... buildid=build.id) |
| 922 | |
| 923 | - >>> pmount_upload = NascentUpload( |
| 924 | + >>> pmount_upload = NascentUpload.from_changesfile_path( |
| 925 | ... datadir('pmount_0.9.7-2ubuntu2_amd64.changes'), |
| 926 | ... buildd_policy, mock_logger) |
| 927 | DEBUG: Changes file can be unsigned. |
| 928 | |
| 929 | === modified file 'lib/lp/soyuz/doc/distroseriesqueue.txt' |
| 930 | --- lib/lp/soyuz/doc/distroseriesqueue.txt 2010-05-14 04:51:42 +0000 |
| 931 | +++ lib/lp/soyuz/doc/distroseriesqueue.txt 2010-08-04 22:12:18 +0000 |
| 932 | @@ -69,7 +69,7 @@ |
| 933 | >>> anything_policy.can_upload_binaries = True |
| 934 | >>> anything_policy.can_upload_mixed = True |
| 935 | |
| 936 | - >>> ed_upload = NascentUpload( |
| 937 | + >>> ed_upload = NascentUpload.from_changesfile_path( |
| 938 | ... datadir('ed_0.2-20_i386.changes'), |
| 939 | ... anything_policy, mock_logger_quiet) |
| 940 | |
| 941 | @@ -263,7 +263,7 @@ |
| 942 | >>> insecure_policy = getPolicy( |
| 943 | ... name='insecure', distro='ubuntu', distroseries='hoary') |
| 944 | |
| 945 | - >>> bar_ok = NascentUpload( |
| 946 | + >>> bar_ok = NascentUpload.from_changesfile_path( |
| 947 | ... datadir('suite/bar_1.0-1/bar_1.0-1_source.changes'), |
| 948 | ... insecure_policy, mock_logger_quiet) |
| 949 | >>> bar_ok.process() |
| 950 | |
| 951 | === modified file 'lib/lp/soyuz/scripts/tests/test_queue.py' |
| 952 | --- lib/lp/soyuz/scripts/tests/test_queue.py 2010-02-09 00:17:40 +0000 |
| 953 | +++ lib/lp/soyuz/scripts/tests/test_queue.py 2010-08-04 22:12:18 +0000 |
| 954 | @@ -123,7 +123,7 @@ |
| 955 | LaunchpadZopelessLayer.switchDbUser("uploader") |
| 956 | sync_policy = getPolicy( |
| 957 | name='sync', distro='ubuntu', distroseries='breezy-autotest') |
| 958 | - bar_src = NascentUpload( |
| 959 | + bar_src = NascentUpload.from_changesfile_path( |
| 960 | datadir(changesfile), |
| 961 | sync_policy, mock_logger_quiet) |
| 962 | bar_src.process() |
Hi William,
This is a meaty change, lots of new code. Very nice. I especially like the refactoring of NascentUpload and _matchDDEBs(): both excellent changes.
Here is the list of notes I took while reviewing the code:
• Do ChangesFile objects have a .path attribute that you can use instead of string-sniffing in the constructor? If so, then perhaps you can push the fiddly conditional construction down into ChangesFile itself and at the same time eliminate the .changesfile_path attribute from the NascentUpload object. A NascentUpload.
• Switching from .reject() to 'yield UploadError()' is a pretty big API change, however I do not see any code in the diff (besides the tests) that is impacted by it. Why not?
• It would be good to have a docstring for _matchDDEBs() that tells you that it is a generator method.
• I think the test names could be improved a bit: the comment should be used as test method name, because the comments tell you what the desired behaviour is. They tell you what the test is actually testing.
• You might be able to rewrite all those "self.assertEqu
With the above points taken into consideration, r=mars