launchpad-reviewers team mailing list archive

[Maximiliano Bertacchini <maximiliano.bertacchini@xxxxxxxxxxxxx>]

Looks good. One question inline.

Diff comments:

> 
> === modified file 'lib/lp/archiveuploader/tests/test_snapupload.py'
> --- lib/lp/archiveuploader/tests/test_snapupload.py	2016-06-28 21:10:18 +0000
> +++ lib/lp/archiveuploader/tests/test_snapupload.py	2016-08-30 12:36:14 +0000
> @@ -62,6 +63,23 @@
>          self.assertEqual(BuildStatus.FULLYBUILT, self.build.status)
>          self.assertTrue(self.build.verifySuccessfulUpload())
>  
> +    def test_requires_snap(self):
> +        # The upload processor fails if the upload does not contain any
> +        # .snap files.
> +        self.assertFalse(self.build.verifySuccessfulUpload())
> +        upload_dir = os.path.join(
> +            self.incoming_folder, "test", str(self.build.id), "ubuntu")
> +        write_file(os.path.join(upload_dir, "wget_0_all.manifest"), "manifest")
> +        handler = UploadHandler.forProcessor(
> +            self.uploadprocessor, self.incoming_folder, "test", self.build)
> +        result = handler.processSnap(self.log)
> +        self.assertEqual(UploadStatusEnum.REJECTED, result)
> +        self.assertIn(
> +            "ERROR Build did not produce any snap packages.",
> +            self.log.getLogBuffer())
> +        self.assertEqual(BuildStatus.UPLOADING, self.build.status)

Is it okay that the build status is still `UPLOADING` here?

> +        self.assertFalse(self.build.verifySuccessfulUpload())
> +
>      def test_triggers_store_uploads(self):
>          # The upload processor triggers store uploads if appropriate.
>          self.pushConfig(


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-upload-check-filename/+merge/304369
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-upload-check-filename into lp:launchpad.