Merge lp:~salgado/launchpad/fix-distro-bug-reporting-test into lp:~launchpad-committers/launchpad/python-migration

[Guilherme Salgado]

[Guilherme Salgado (salgado)]

Looks like the librarian socket, in 2.5, is garbage collected right after an
upload, thus breaking test_re_upload_file_does_not_leak_file_descriptors. This
changes the test to not expect the socket to be open. We should probably not
rely on the garbage collector to do this, but fixing the librarian is out of
scope for this.

[Gary Poster (gary)]

merge-conditional

Hi Salgado. This looks good.

It's not obvious looking at the test in isolation that the re-upload actually worked. You told me on IRC that there is an assertion in getUtility(ILibraryFileAliasSet).create() that the file is created, so we are covered practically. I'd just like a comment in that regard in the test.

Thank you

Gary

review: Approve

[Guilherme Salgado (salgado)]

Thanks Gary, this is the change I did to the test.

=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py 2009-10-16 15:11:05 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py 2009-10-16 16:41:07 +0000
@@ -161,6 +161,9 @@
         previously_open_files = number_of_open_files()

         public_file = re_upload_file(private_file)
+ # The above call would've raised an error if the upload failed, but
+ # better safe than sorry.
+ self.assertIsNot(None, public_file)

         open_files = number_of_open_files() - previously_open_files
         self.assertEqual(0, open_files)

=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py	2009-09-29 17:27:52 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py	2009-10-16 15:20:27 +0000
@@ -152,9 +152,7 @@
         self.assertFileIsReset(private_file)
 
     def test_re_upload_file_does_not_leak_file_descriptors(self):
-        # Reuploading a library file doesn't leak file descriptors. The
-        # only extra file opened by the end of the process is the socket
-        # with the librarian server.
+        # Reuploading a library file doesn't leak file descriptors.
         private_file = self.factory.makeLibraryFileAlias(restricted=True)
         transaction.commit()
 
@@ -165,7 +163,7 @@
         public_file = re_upload_file(private_file)
 
         open_files = number_of_open_files() - previously_open_files
-        self.assertEqual(1, open_files)
+        self.assertEqual(0, open_files)
 
 
 class UpdateFilesPrivacyTestCase(TestCaseWithFactory):