Code review comment for lp:~nmb/bzr/662509-ignore-blanks

Revision history for this message
Vincent Ladeuil (vila) wrote :

s/ignore_blanks/ignore_output/ is almost my only comment.

This is exactly how I wanted to implement it.

When we add keywords arguments, we try to also use them as such so:

38 + def run_script(self, script, ignore_blanks=False):
39 + return self.script_runner.run_script(self, script, ignore_blanks)

ignore_output=ignore_output

The idea is that if we add more mandatory arguments the above (erroneous) call *may* fail later, whereas using the keyword form ensures that it fails right there, at the call.

49 + def run_script(self, script, ignore_blanks=False):
50 + return self.script_runner.run_script(self, script, ignore_blanks)

60 + return ScriptRunner().run_script(test_case, script_string, ignore_blanks)

idem

74 +If output occurs and no output is expected, the execution stops and the
75 +test fails. If unexpected output occurs on the standard error, then
76 +execution stops.

Good catch !

So ignore_output=False can even be check_output=True

 +irrelevant, the run_script() method may be passed the keyword argument
101 +``ignore_blanks=True``. For example::
102 +
103 + def test_ignoring_blank_output(self):

Don't forget to rename here too.

I let poolie be the second reviewer here but my 'NeedsFixing' vote is really bb:tweak.

review: Needs Fixing

« Back to merge proposal