canonical-ubuntu-qa team mailing list archive

[Tim Andersson <mp+469288@xxxxxxxxxxxxxxxxxx>]

Tim Andersson has proposed merging ~andersson123/autopkgtest-cloud:fix-ci-increase-lp-request-timeout into autopkgtest-cloud:master.

Commit message:
# Commit 1:

fix: web: request/submit.py: increase timeout in lp_request

This change should help with the 500's people have been recently
encountering in production.

I believe a number of the 500s people have been encountering have been
due to the lp_request function timing out. The timeout was previously 10
seconds and I've thusly bumped it to 30. A bit of a liberal timeout, but
I think it's better to err on this side. I obviously have no logs to
support my theory that this timeout was the result of a lot of 500s,
since we have our Exception eater, but I have a hunch.

# Commit 2:
fix: web: don't let unit tests make api calls to launchpad

This commit refactors a couple of unit tests which weren't mocking
the urllib.url_open function call - resulting in our unit tests in CI
actually making api calls to Launchpad, which isn't appropriate. They
now correctly mock the url_open call and no longer make queries to the
Launchpad API.

This explains our recent flaky CI - perhaps there was some Launchpad
instability, or at least, something causing queries to take a little
longer, which was in turn causing our unit tests to fail.

Requested reviews:
  Canonical's Ubuntu QA (canonical-ubuntu-qa)

For more details, see:
https://code.launchpad.net/~andersson123/autopkgtest-cloud/+git/autopkgtest-cloud/+merge/469288

Fixes our flaky CI failures, which were a result of our unit tests actually making queries to the Launchpad API, and timing out.

Also increases the timeout in the lp_request function, as Tim believes this is likely the cause of at least some of the 500s people have been reporting
-- 
Your team Canonical's Ubuntu QA is requested to review the proposed merge of ~andersson123/autopkgtest-cloud:fix-ci-increase-lp-request-timeout into autopkgtest-cloud:master.

diff --git a/charms/focal/autopkgtest-web/webcontrol/request/submit.py b/charms/focal/autopkgtest-web/webcontrol/request/submit.py
index 84b7425..316d15e 100644
--- a/charms/focal/autopkgtest-web/webcontrol/request/submit.py
+++ b/charms/focal/autopkgtest-web/webcontrol/request/submit.py
@@ -550,7 +550,7 @@ class Submit:
         """
         url = LP + obj + "?" + urllib.parse.urlencode(query)
         try:
-            with urllib.request.urlopen(url, timeout=10) as req:
+            with urllib.request.urlopen(url, timeout=30) as req:
                 code = req.getcode()
                 if code >= 300:
                     logging.error(
diff --git a/charms/focal/autopkgtest-web/webcontrol/request/tests/test_submit.py b/charms/focal/autopkgtest-web/webcontrol/request/tests/test_submit.py
index 5fefef5..2c6a65d 100644
--- a/charms/focal/autopkgtest-web/webcontrol/request/tests/test_submit.py
+++ b/charms/focal/autopkgtest-web/webcontrol/request/tests/test_submit.py
@@ -89,8 +89,21 @@ class DistroRequestValidationTests(SubmitTestBase):
             )
         self.assertEqual(str(cme.exception), "arch wut not found")
 
-    def test_bad_package(self):
+    @patch("request.submit.urllib.request.urlopen")
+    def test_bad_package(self, mock_urlopen):
         """Unknown package"""
+        cm = MagicMock()
+        cm.__enter__.return_value = cm
+        cm.getcode.return_value = 200
+        cm.geturl.return_value = "http://mock.launchpad.net";
+        cm.read.side_effect = [
+            b'{"entries": []}',
+            b'{"total_size": 0}',
+            b'{"entries": []}',
+            b'{"total_size": 0}',
+        ]
+        cm.return_value = cm
+        mock_urlopen.return_value = cm
 
         with self.assertRaises(WebControlException) as cme:
             self.submit.validate_distro_request(
@@ -107,8 +120,20 @@ class DistroRequestValidationTests(SubmitTestBase):
             )
         self.assertIn("Invalid argument foo", str(cme.exception))
 
-    def test_invalid_trigger_syntax(self):
+    @patch("request.submit.urllib.request.urlopen")
+    def test_invalid_trigger_syntax(self, mock_urlopen):
         """Invalid syntax in trigger"""
+        cm = MagicMock()
+        cm.__enter__.return_value = cm
+        cm.getcode.return_value = 200
+        cm.geturl.return_value = "http://mock.launchpad.net";
+        cm.read.side_effect = [
+            b'{"entries": []}',
+            b'{"entries": []}',
+            b'{"entries": []}',
+        ]
+        cm.return_value = cm
+        mock_urlopen.return_value = cm
 
         # invalid trigger format
         with self.assertRaises(WebControlException) as cme: