Re: Stylistic question regarding no-op code and tests
On Wed, Oct 14, 2015 at 10:07 PM, Ben Finney wrote: > Jason Swails writes: > > > What I recently realized, though, that what this construct allows is > > for the coverage testing package (which I have recently started > > employing for my project... thanks Ned and others!) to detect whether > > or not both code paths are covered in the test suite. > > Coverage.py has, for many releases now, had good measurement of branch > coverage by your tests. Enable it with the ‘--branch’ option to ‘run’ > Oh, cool. I'm actually using coverage indirectly through nose, so I haven't really looked through the coverage docs (although nosetests has a --cover-branches option that toggles this feature). Now I can go back to cringing about "else: pass" in peace :). Thanks! Jason -- Jason M. Swails BioMaPS, Rutgers University Postdoctoral Researcher -- https://mail.python.org/mailman/listinfo/python-list
Re: Stylistic question regarding no-op code and tests
Jason Swails wrote: > Hi everyone, > > I'd like to get some opinions about some coding constructs that may seem > at first glance to serve no purpose, but does have *some* non-negligible > purpose, and I think I've come to the right place :). > > The construct is this: > > def my_function(arg1, arg2, filename=None): > """ Some function. If a file is given, it is processed """ > # Some code that performs my_function > if filename is not None: > process_file(filename) > else: > pass > > My question is, what do you think of the "else: pass" statement? It is a > complete no-op and is syntactically equivalent to the same code with those > lines removed. Up until earlier today, I would look at that and cringe (I > still do, a little). > > What I recently realized, though, that what this construct allows is for > the coverage testing package (which I have recently started employing for > my project... thanks Ned and others!) to detect whether or not both code > paths are covered in the test suite. > > I think my opinion here is that this construct is useful to use when the > two code paths are very different operationally from each other, one is an > unusual path that you are not >100% sure is well-covered in your test > suite, but that your first instinct should be to avoid such code. > > What do you think? Rewrite my_function() in such a way that you can devise unit tests that verify the process_file path is taken if and only if filename is not None. The minimal change would be def my_function(arg1, arg2, filename=None): """ Some function. If a file is given, it is processed """ # Some code that performs my_function if filename is not None: process_file(filename) return True else: return False but there are certainly options that better reflect the actual purpose of my_function(). coverage.py is a nice addition, but tests are the first line of defense against buggy code. -- https://mail.python.org/mailman/listinfo/python-list
Re: Stylistic question regarding no-op code and tests
Jason Swails writes: > What I recently realized, though, that what this construct allows is > for the coverage testing package (which I have recently started > employing for my project... thanks Ned and others!) to detect whether > or not both code paths are covered in the test suite. Coverage.py has, for many releases now, had good measurement of branch coverage by your tests. Enable it with the ‘--branch’ option to ‘run’ https://coverage.readthedocs.org/en/latest/branch.html>. -- \ “They who can give up essential liberty to obtain a little | `\temporary safety, deserve neither liberty nor safety.” | _o__) —Benjamin Franklin, 1775-02-17 | Ben Finney -- https://mail.python.org/mailman/listinfo/python-list
Re: Stylistic question regarding no-op code and tests
On Thu, Oct 15, 2015 at 12:49 PM, Jason Swails wrote: > My question is, what do you think of the "else: pass" statement? It is a > complete no-op and is syntactically equivalent to the same code with those > lines removed. Up until earlier today, I would look at that and cringe (I > still do, a little). > > What I recently realized, though, that what this construct allows is for the > coverage testing package (which I have recently started employing for my > project... thanks Ned and others!) to detect whether or not both code paths > are covered in the test suite. If that's the case, it absolutely MUST have some acknowledgement in the code. Maybe a comment, or maybe replace the 'pass' with an 'assert', or (a neat trick I've used in a few places) a statistical counter: if filename is not None: process_file(filename) files_processed += 1 else: nonfiles_not_processed += 1 Your coverage tester should be just as happy with that, and it doesn't look pointless. ChrisA -- https://mail.python.org/mailman/listinfo/python-list
