Code review comment for lp:~julian-edwards/launchpad/hanging-builder-edit-bug-583905

Revision history for this message
Michael Nelson (michael.nelson) wrote :

16:40 < noodles775> bigjools: why do you remove the proxy on the test case's instance var, rather than just when you need to set something that you normally wouldn't be able to?
16:45 < noodles775> bigjools: also, you could use FakeMethod there if you wanted to... it would mean that you don't need to set slave_called on the builder, which would also mean you don't need removeSecurityProxy right?
16:46 < bigjools> noodles775: proxy is removed because...ummm I can't remember, let me try without it again since I've changed the code a bit since I added it (for good reason at the time)
16:47 < noodles775> I'm guessing it's because you're setting builder.slave_called, which isn't a interface attribute (well, it's not an attribute at all except for your test is it?)
16:47 < bigjools> indeed
16:47 < bigjools> ah, it's because I can't override slaveStatusSentence otherwise
16:48 < bigjools> well, I suppose I could name it the same
16:49 < noodles775> Could you do the following in setUp: removeSecurityProxy(self.builder).slaveStatusSentence = FakeMethod(...)
16:50 < noodles775> Then your test could just check self.builder.slaveStatusSentence.call_count == 0
16:50 < bigjools> yeah, let me try it, I wasn't aware of FakeMethod
16:51 < noodles775> OK, with that r=me

review: Approve (code)

« Back to merge proposal