----- "Amos Kong" <akong@xxxxxxxxxx> wrote: > On Fri, Dec 25, 2009 at 08:28:18AM -0500, Michael Goldish wrote: > > > > ----- "Amos Kong" <akong@xxxxxxxxxx> wrote: > > > > > If login timeout, wait_for() returned 'None' and assigned to > > > 'session'. > > > When call session.close(), this prlblem was caused: > > > "AttributeError: 'NoneType' object has no attribute 'close'" > > > > > > Signed-off-by: Amos Kong <akong@xxxxxxxxxx> > > > --- > > > client/tests/kvm/tests/timedrift_with_migration.py | 3 ++- > > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > > > diff --git a/client/tests/kvm/tests/timedrift_with_migration.py > > > b/client/tests/kvm/tests/timedrift_with_migration.py > > > index a012db3..0b93183 100644 > > > --- a/client/tests/kvm/tests/timedrift_with_migration.py > > > +++ b/client/tests/kvm/tests/timedrift_with_migration.py > > > @@ -76,7 +76,8 @@ def run_timedrift_with_migration(test, params, > > > env): > > > time_filter_re, > > > time_format) > > > > > > finally: > > > - session.close() > > > + if session != None: > > > + session.close() > > > > Agreed, but we can make this simply: > > > > if session: > > session.close() > > Yes, > > > > There's no need to explicitly check for None (and if there was, > > the preferred syntax would be 'is not None' rather than '!= None'). > > > > Also, just to be safe, we should make the same modification to > > timedrift_with_reboot.py. > > > In timedrift_with_reboot.py, 'session' has been assigned before 'try'. > If re-login timout, kvm_test_utils.reboot() returns nothing, the value > of 'session' isn't changed. > So session.close() couldn't cause this problem: > "AttributeError: 'NoneType' object has no attribute 'close'" The two tests are nearly identical so I thought we might as well make the change in both of them, but I agree that it doesn't matter (unless we change the behavior of kvm_test_utils.reboot() in the future). > > > > In other testcases, if session wasn't assigned before 'try', > when calling kvm_test_utils.wait_for_login()/kvm_test_utils.reboot() > timeout, > It returns nothing, if close 'session' in finally part, Another > problem will occur: > "NameError: name 'session' is not defined" > > In this condition, > """ > if session: > session.close() > """ > also causes this error. In what tests exactly does this happen? 'if session' is preferable to 'if session is not None' because it's shorter, not because it's safer. > > > > > We can also consider removing the try..finally clauses altogether > > because sessions are now closed automatically when they're no > longer > > needed. > > > > > > > > # Report results > > > host_delta = ht1 - ht0 > > > -- > > > 1.5.5.6 > > -- > Amos Kong > Quality Engineer > Raycom Office(Beijing), Red Hat Inc. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
