Code review comment for lp://staging/~tealeg/landscape-client/package-monitor-scoped-resynch

Revision history for this message
Björn Tillenius (bjornt) wrote :

Looks good, +1

[1]

+ def test_not_resynchronize_with_other_scope(self):
+ """
+ If a 'resynchronize' reactor event is fired with an irrelevant scope,
+ the package monitor should not respond to this.
+ """
+ self.monitor.add(self.package_monitor)
+ self.createReporterTask()
+
+ disk_scope = ["disk"]
+ self.monitor.reactor.fire("resynchronize", disk_scope)
+
+ # The next task should *not* be the resynchronize message.
+ self.assertSingleReporterTask(
+ {'ids': [None], 'request-id': 1, 'type': 'package-ids'}, 1)

Where does that dict on the last line come from? Ok, after reading the
diff I can see that it's from createReporterTask(), but if someone looks
at the test only, it's hard to see.

It would be better to either explicitly add that task you expect to be
there, or make createReportTask() return the task and id, so that it's
clear that you expect the existing task to still be there, and no a new
one.

review: Approve

« Back to merge proposal