launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/bug-865892 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #865892 in Launchpad itself: "smoke-test-librarian.py doesn't test the full download config"
  https://bugs.launchpad.net/launchpad/+bug/865892

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-865892/+merge/78045

Because duplicated configuration is fun, each librarian download endpoint is configured twice: (restricted_)?download_(host|port) and (restricted_)?download_url. This branch fixes the librarian smoketest to check both, after the recent production amusement where one was configured correctly, while the other had the wrong port.
-- 
https://code.launchpad.net/~wgrant/launchpad/bug-865892/+merge/78045
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bug-865892 into lp:launchpad.

=== modified file 'lib/canonical/librarian/smoketest.py'
--- lib/canonical/librarian/smoketest.py	2010-12-24 11:46:18 +0000
+++ lib/canonical/librarian/smoketest.py	2011-10-04 06:00:26 +0000
@@ -30,7 +30,7 @@
     # To be able to retrieve the file, we must commit the current transaction.
     transaction.commit()
     alias = getUtility(ILibraryFileAliasSet)[file_id]
-    return alias.http_url
+    return (file_id, alias.http_url)
 
 
 def read_file(url):
@@ -47,20 +47,27 @@
     return data
 
 
+def upload_and_check(client, output):
+    id, url = store_file(client)
+    output.write('retrieving file from http_url (%s)\n' % (url,))
+    if read_file(url) != FILE_DATA:
+        return False
+    output.write('retrieving file from client\n')
+    if client.getFileByAlias(id).read() != FILE_DATA:
+        return False
+    return True
+
+
 def do_smoketest(restricted_client, regular_client, output=None):
     if output is None:
         output = sys.stdout
     output.write('adding a private file to the librarian...\n')
-    private_url = store_file(restricted_client)
-    output.write('retrieving private file from %s\n' % (private_url,))
-    if read_file(private_url) != FILE_DATA:
+    if not upload_and_check(restricted_client, output):
         output.write('ERROR: data fetched does not match data written\n')
         return 1
 
     output.write('adding a public file to the librarian...\n')
-    public_url = store_file(regular_client)
-    output.write('retrieving public file from %s\n' % (public_url,))
-    if read_file(public_url) != FILE_DATA:
+    if not upload_and_check(regular_client, output):
         output.write('ERROR: data fetched does not match data written\n')
         return 1
 

=== modified file 'lib/canonical/librarian/tests/test_smoketest.py'
--- lib/canonical/librarian/tests/test_smoketest.py	2010-12-24 11:46:18 +0000
+++ lib/canonical/librarian/tests/test_smoketest.py	2011-10-04 06:00:26 +0000
@@ -71,7 +71,7 @@
         # and return the file's HTTP URL works.
         self.assertEquals(
             store_file(self.fake_librarian),
-            'http://localhost:58000/93/smoke-test-file')
+            (93, 'http://localhost:58000/93/smoke-test-file'))
 
     def test_good_data(self):
         # If storing and retrieving both the public and private files work,