GNOME Bugzilla – Bug 744682
tests: session vms backup cleanup
Last modified: 2016-03-31 13:22:07 UTC
* function created for do_backup and do_restore * one regression fixed in snapshot restoring * restoring machines even when vnc and system-brokers are skipped
Created attachment 297059 [details] tests: backup and restore cleanups * Backup and restore functions created so they can be used before exit(77) when skipping system broker or vnc tests in case of missing environmental needs. * Change directory regression fixed in restoring snapshots.
Review of attachment 297059 [details]: There is at least two different logical changes here so can we please divide it?
Created attachment 297146 [details] [review] tests: Cleaning restore-backup code Move all saving and restoring code into separate functions.
Created attachment 297148 [details] [review] tests: Directory change fix in restore function Going back to original directory after every machine snaphsots restored.
Created attachment 297149 [details] [review] tests: Restoring VMs after scenario skip Machines are now restored even after skipped scenarios.
Created attachment 297150 [details] [review] tests: Remove both SB machines Deleting both system broker machines that are used in import 2 boxes from system broker test.
Created attachment 297153 [details] [review] tests: Refactor backup & restoring code Move all saving and restoring code into separate functions.
Created attachment 297154 [details] [review] tests: Change directory outside & after loop Go back to original directory after all machine snaphsots are restored.
Created attachment 297155 [details] [review] tests: Restore VMs when scenario are skipped Ensure machines are now restored even after skipped scenarios.
Created attachment 297156 [details] [review] tests: Clean-up system broker VMs Delete both machines that are created by 'Import 2 box from system broker' scenario from system broker.
Attachment 297153 [details] pushed as 630e3ea - tests: Refactor backup & restoring code Attachment 297154 [details] pushed as 3515109 - tests: Change directory outside & after loop Attachment 297155 [details] pushed as bac6c26 - tests: Restore VMs when scenario are skipped Attachment 297156 [details] pushed as 3bd95c9 - tests: Clean-up system broker VMs
Review of attachment 297156 [details] [review]: ::: tests/environment.py @@ +219,3 @@ + os.system('virsh -q -c qemu:///system destroy Core-5.3-2 > /dev/null 2>&1') + os.system('virsh -q -c qemu:///system undefine Core-5.3-2 > /dev/null 2>&1') + While I pushed this to include in the pending release, I think this should either by done by the scenario itself or this should be made generic.
(In reply to Zeeshan Ali (Khattak) from comment #12) > Review of attachment 297156 [details] [review] [review]: > > ::: tests/environment.py > @@ +219,3 @@ > + os.system('virsh -q -c qemu:///system destroy Core-5.3-2 > > /dev/null 2>&1') > + os.system('virsh -q -c qemu:///system undefine Core-5.3-2 > > /dev/null 2>&1') > + > > While I pushed this to include in the pending release, I think this should > either by done by the scenario itself or this should be made generic. These deletions are wanted to be done outside of test itself as they're not a part of it and failing in deletion is not something you may want in test itself. This is exactly what environment.py should be used for. You don't want to delete any other machine that was created by test itself as we are not touching vms that existed in system broker. What I can think of is to delete anything that has Core-5.3 inside.
(In reply to vladimir benes from comment #13) > (In reply to Zeeshan Ali (Khattak) from comment #12) > > Review of attachment 297156 [details] [review] [review] [review]: > > > > ::: tests/environment.py > > @@ +219,3 @@ > > + os.system('virsh -q -c qemu:///system destroy Core-5.3-2 > > > /dev/null 2>&1') > > + os.system('virsh -q -c qemu:///system undefine Core-5.3-2 > > > /dev/null 2>&1') > > + > > > > While I pushed this to include in the pending release, I think this should > > either by done by the scenario itself or this should be made generic. > > These deletions are wanted to be done outside of test itself as they're not > a part of it and failing in deletion is not something you may want in test > itself. This is exactly what environment.py should be used for. > > You don't want to delete any other machine that was created by test itself > as we are not touching vms that existed in system broker. What I can think > of is to delete anything that has Core-5.3 inside. Of course not but we create these machines and give them names so we can give them unique names (e.g boxes-tests-Core-5.3) to ensure that we don't end up deleting some other machines.