DaanHoogland commented on code in PR #10583:
URL: https://github.com/apache/cloudstack/pull/10583#discussion_r2009986890


##########
test/integration/smoke/test_network_permissions.py:
##########
@@ -733,6 +733,11 @@ def 
test_04_deploy_vm_for_other_user_and_test_vm_operations(self):
         self.exec_command("self.user_apiclient", command, expected=False)
         self.exec_command("self.otheruser_apiclient", command, expected=True)
 
+        #22. Start VM before destroying, to recreate ROOT volume that was 
deleted as part of restore operation
+        command = """self.virtual_machine.start({apiclient})"""
+        self.exec_command("self.user_apiclient", command, expected=False)
+        self.exec_command("self.otheruser_apiclient", command, expected=True)
+

Review Comment:
   a second step number 22 ! would it make sense to split this test? It already 
has 26 steps , adn I see some doing double work as well.
   Is this creating extra value to our testing?



##########
test/integration/smoke/test_events_resource.py:
##########
@@ -161,21 +157,12 @@ def test_01_events_resource(self):
         virtual_machine.restore(self.apiclient)
         time.sleep(self.services["sleep"])
         virtual_machine.detach_volume(self.apiclient, volume)
-        volume.delete(self.apiclient)
-        self.cleanup.remove(volume)
         ts = str(time.time())
         virtual_machine.update(self.apiclient, displayname=ts)
-        virtual_machine.delete(self.apiclient)
-        self.cleanup.remove(virtual_machine)
         account_network.update(self.apiclient, name=account_network.name + ts)
-        account_network.delete(self.apiclient)
-        self.cleanup.remove(account_network)
+        virtual_machine.start(self.apiclient)

Review Comment:
   I see your point and the point of such tests, but we are not checking for 
specific events anyway, just listing them. If the test is error prone, not 
specific and sensitive to environmental issues, I'd rather we simplify it and 
add more specific checks. Would you agree?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to