launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/refactor-test-bugtaskfilter into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/refactor-test-bugtaskfilter/+merge/192456

When I was investigating a test failure, I came across this file. Refactor it within an inch of its life, removing a lot of duplication into a nice and neat custom assert method.
-- 
https://code.launchpad.net/~stevenk/launchpad/refactor-test-bugtaskfilter/+merge/192456
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/refactor-test-bugtaskfilter into lp:launchpad.

=== modified file 'lib/lp/bugs/tests/test_bugtaskfilter.py'
--- lib/lp/bugs/tests/test_bugtaskfilter.py	2012-08-08 07:22:51 +0000
+++ lib/lp/bugs/tests/test_bugtaskfilter.py	2013-10-24 04:05:54 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2011-2013 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for lp.bugs.interfaces.bugtaskfilter."""
@@ -23,9 +23,7 @@
     def test_simple_case(self):
         bug = self.factory.makeBug()
         tasks = list(bug.bugtasks)
-        self.assertThat(
-            filter_bugtasks_by_context(None, tasks),
-            Equals(tasks))
+        self.assertThat(filter_bugtasks_by_context(None, tasks), Equals(tasks))
 
     def test_multiple_bugs(self):
         bug1 = self.factory.makeBug()
@@ -40,38 +38,34 @@
         self.assertThat(len(filtered), Equals(3))
         self.assertThat(filtered, Equals(tasks))
 
+    def assertFilterBugtasksByContextNoQueries(self, bug, target, task):
+        tasks = list(bug.bugtasks)
+        with StormStatementRecorder() as recorder:
+            filtered = filter_bugtasks_by_context(target, tasks)
+        self.assertThat(recorder, HasQueryCount(Equals(0)))
+        self.assertThat(filtered, Equals([task]))
+
     def test_two_product_tasks_case_no_context(self):
         widget = self.factory.makeProduct()
         bug = self.factory.makeBug(target=widget)
         cogs = self.factory.makeProduct()
         self.factory.makeBugTask(bug=bug, target=cogs)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(None, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([bug.getBugTask(widget)]))
+        self.assertFilterBugtasksByContextNoQueries(
+            bug, None, bug.getBugTask(widget))
 
     def test_two_product_tasks_case(self):
         widget = self.factory.makeProduct()
         bug = self.factory.makeBug(target=widget)
         cogs = self.factory.makeProduct()
         task = self.factory.makeBugTask(bug=bug, target=cogs)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(cogs, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, cogs, task)
 
     def test_product_context_with_series_task(self):
         bug = self.factory.makeBug()
         widget = self.factory.makeProduct()
         task = self.factory.makeBugTask(bug=bug, target=widget)
         self.factory.makeBugTask(bug=bug, target=widget.development_focus)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(widget, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, widget, task)
 
     def test_productseries_context_with_series_task(self):
         bug = self.factory.makeBug()
@@ -79,32 +73,20 @@
         self.factory.makeBugTask(bug=bug, target=widget)
         series = widget.development_focus
         task = self.factory.makeBugTask(bug=bug, target=series)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(series, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, series, task)
 
     def test_productseries_context_with_only_product_task(self):
         bug = self.factory.makeBug()
         widget = self.factory.makeProduct()
         task = self.factory.makeBugTask(bug=bug, target=widget)
         series = widget.development_focus
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(series, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, series, task)
 
     def test_distro_context(self):
         bug = self.factory.makeBug()
         mint = self.factory.makeDistribution()
         task = self.factory.makeBugTask(bug=bug, target=mint)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(mint, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, mint, task)
 
     def test_distro_context_with_series_task(self):
         bug = self.factory.makeBug()
@@ -112,11 +94,7 @@
         task = self.factory.makeBugTask(bug=bug, target=mint)
         devel = self.factory.makeDistroSeries(mint)
         self.factory.makeBugTask(bug=bug, target=devel)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(mint, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, mint, task)
 
     def test_distroseries_context_with_series_task(self):
         bug = self.factory.makeBug()
@@ -124,63 +102,39 @@
         self.factory.makeBugTask(bug=bug, target=mint)
         devel = self.factory.makeDistroSeries(mint)
         task = self.factory.makeBugTask(bug=bug, target=devel)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(devel, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, devel, task)
 
     def test_distroseries_context_with_no_series_task(self):
         bug = self.factory.makeBug()
         mint = self.factory.makeDistribution()
         task = self.factory.makeBugTask(bug=bug, target=mint)
         devel = self.factory.makeDistroSeries(mint)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(devel, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, devel, task)
 
     def test_sourcepackage_context_with_sourcepackage_task(self):
         bug = self.factory.makeBug()
         sp = self.factory.makeSourcePackage()
         task = self.factory.makeBugTask(bug=bug, target=sp)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(sp, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, sp, task)
 
     def test_sourcepackage_context_with_distrosourcepackage_task(self):
         bug = self.factory.makeBug()
         sp = self.factory.makeSourcePackage()
         dsp = sp.distribution_sourcepackage
         task = self.factory.makeBugTask(bug=bug, target=dsp)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(sp, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, sp, task)
 
     def test_sourcepackage_context_series_task(self):
         bug = self.factory.makeBug()
         sp = self.factory.makeSourcePackage()
         task = self.factory.makeBugTask(bug=bug, target=sp.distroseries)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(sp, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, sp, task)
 
     def test_sourcepackage_context_distro_task(self):
         bug = self.factory.makeBug()
         sp = self.factory.makeSourcePackage()
         task = self.factory.makeBugTask(bug=bug, target=sp.distribution)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(sp, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, sp, task)
 
     def test_sourcepackage_context_distro_task_with_other_distro_package(self):
         bug = self.factory.makeBug()
@@ -189,8 +143,4 @@
         other_sp = self.factory.makeSourcePackage(
             sourcepackagename=sp.sourcepackagename)
         self.factory.makeBugTask(bug=bug, target=other_sp)
-        tasks = list(bug.bugtasks)
-        with StormStatementRecorder() as recorder:
-            filtered = filter_bugtasks_by_context(sp, tasks)
-        self.assertThat(recorder, HasQueryCount(Equals(0)))
-        self.assertThat(filtered, Equals([task]))
+        self.assertFilterBugtasksByContextNoQueries(bug, sp, task)