> On July 25, 2013, 1:54 p.m., Alan Conway wrote: > > trunk/qpid/cpp/src/tests/acl.py, line 1254 > > <https://reviews.apache.org/r/12875/diff/2/?file=326059#file326059line1254> > > > > What's wrong with the original code? I've used this pattern myself, > > often. I don't see any benefit to visiting every use of fail() in the > > tests!!
Just looked at the JIRA: the example there IS wrong, because "catch Exception" will catch the exception thrown by fail(). But this code is correct: except qpid.session.SessionException, e will not catch the exception thrown by fail(). - Alan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12875/#review23841 ----------------------------------------------------------- On July 23, 2013, 6:19 p.m., Chug Rolke wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12875/ > ----------------------------------------------------------- > > (Updated July 23, 2013, 6:19 p.m.) > > > Review request for qpid. > > > Bugs: qpid-5010 > https://issues.apache.org/jira/browse/qpid-5010 > > > Repository: qpid > > > Description > ------- > > Acl.py self tests issue self.fail() calls if an expected exception is not > thrown. A proposed fix for two patterns is shown in this review. > > Essentially if the expected exception is thrown then pass. If some other > exception is thrown then show it's details. If no exception is thrown then > issue the self.fail() that was in the try block. Finally, where necessary, > run the cleanup handling to recover from the exception. > > > Diffs > ----- > > trunk/qpid/cpp/src/tests/acl.py 1506161 > > Diff: https://reviews.apache.org/r/12875/diff/ > > > Testing > ------- > > There are 222 self.fail() calls that need to be examined and two of them are > changed in this review. The rest need scrutiny and when fixed may reveal > failures in the code as in QPID-5011. > > > Thanks, > > Chug Rolke > >
