Addressed Review comments

1. Removed defining test params based per arch and moved them to place they are set ex: cpu_threads & machine_type

2. Removed skip in the code based on arch, rather added s390x  marker to test

Signed-off-by: chandramerla <Chandra.Merla@ibm.com>

Author: chandramerla

tests/conftest.py

@@ -70,7 +70,7 @@
 from timeout_sampler import TimeoutSampler
 
 import utilities.hco
-from tests.utils import download_and_extract_tar, update_cluster_cpu_model
+from utilities.architecture import is_cluster_s390x
 from utilities.bitwarden import get_cnv_tests_secret_by_name
 from utilities.constants import (
     AAQ_NAMESPACE_LABEL,
@@ -107,7 +107,6 @@
     RHEL9_STR,
     RHEL_WITH_INSTANCETYPE_AND_PREFERENCE,
     RHSM_SECRET_NAME,
-    S390X,
     SSP_CR_COMMON_TEMPLATES_LIST_KEY_NAME,
     TIMEOUT_3MIN,
     TIMEOUT_4MIN,
@@ -215,6 +214,9 @@
     wait_for_windows_vm,
 )
 
+from .utils import download_and_extract_tar, update_cluster_cpu_model
+from .virt.constants import MachineTypesNames
+
 LOGGER = logging.getLogger(__name__)
 HTTP_SECRET_NAME = "htpass-secret-for-cnv-tests"
 HTPASSWD_PROVIDER_DICT = {
@@ -2769,12 +2771,22 @@ def cluster_modern_cpu_model_scope_class(
     wait_for_kv_stabilize(admin_client=admin_client, hco_namespace=hco_namespace)
 
 
+@pytest.fixture(scope="session")
+def is_s390x_cluster():
+    return is_cluster_s390x()
+
+
+@pytest.fixture(scope="function")
+def non_default_machine_type(is_s390x_cluster):
+    return MachineTypesNames.s390_ccw_virtio if is_s390x_cluster else MachineTypesNames.pc_q35_rhel7_6
+
+
 @pytest.fixture(scope="module")
-def machine_type_from_kubevirt_config(kubevirt_config_scope_module, nodes_cpu_architecture):
+def machine_type_from_kubevirt_config(is_s390x_cluster, kubevirt_config_scope_module, nodes_cpu_architecture):
     """Extract machine type default from kubevirt CR."""
     # Workaround for s390x (https://github.com/kubevirt/kubevirt/issues/14953), as machine type missing in config and
     # hardcoded to s390_ccw_virtio in kubevirt code.
-    if nodes_cpu_architecture == S390X:
+    if is_s390x_cluster:
         mc_type = "s390-ccw-virtio"
     else:
         mc_type = kubevirt_config_scope_module["architectureConfiguration"][nodes_cpu_architecture]["machineType"]

tests/utils.py

@@ -284,7 +284,7 @@ def update_cluster_cpu_model(admin_client, hco_namespace, hco_resource, cpu_mode
             hco_namespace=hco_namespace,
             path=["cpuModel"],
             value=cpu_model,
-            timeout=120,
+            timeout=30,
         )
         yield
 

tests/virt/node/conftest.py

@@ -17,7 +17,6 @@
     FOUR_GI_MEMORY,
     ONE_CPU_CORE,
     ONE_CPU_THREAD,
-    S390X,
     TEN_GI_MEMORY,
 )
 from utilities.virt import (
@@ -56,7 +55,7 @@ def hotplugged_vm(
     unprivileged_client,
     golden_image_data_source_scope_class,
     modern_cpu_for_migration,
-    nodes_cpu_architecture,
+    is_s390x_cluster,
 ):
     with VirtualMachineForTestsFromTemplate(
         name=request.param["vm_name"],
@@ -66,8 +65,8 @@ def hotplugged_vm(
         client=unprivileged_client,
         data_source=golden_image_data_source_scope_class,
         cpu_max_sockets=EIGHT_CPU_SOCKETS,
-        # s390x doesn't support maxGuest as it doesn't support hotplug
-        memory_max_guest=TEN_GI_MEMORY if nodes_cpu_architecture != S390X else None,
+        # s390x doesn't support maxGuest as it doesn't support hotplug memory
+        memory_max_guest=None if is_s390x_cluster else TEN_GI_MEMORY,
         cpu_sockets=FOUR_CPU_SOCKETS,
         cpu_threads=ONE_CPU_THREAD,
         cpu_cores=ONE_CPU_CORE,

tests/virt/node/cpu_sockets_threads/test_cpu_support_sockets_threads.py

@@ -21,7 +21,7 @@ def check_vm_dumpxml(vm, cores=None, sockets=None, threads=None):
 
 
 @pytest.fixture()
-def vm_with_cpu_support(request, namespace, unprivileged_client):
+def vm_with_cpu_support(request, is_s390x_cluster, namespace, unprivileged_client):
     """
     VM with CPU support (cores,sockets,threads)
     """
@@ -31,7 +31,7 @@ def vm_with_cpu_support(request, namespace, unprivileged_client):
         namespace=namespace.name,
         cpu_cores=request.param["cores"],
         cpu_sockets=request.param["sockets"],
-        cpu_threads=request.param["threads"],
+        cpu_threads=1 if is_s390x_cluster else request.param["threads"],
         cpu_max_sockets=request.param["sockets"] or 1,
         body=fedora_vm_body(name=name),
         client=unprivileged_client,
@@ -45,37 +45,28 @@ def vm_with_cpu_support(request, namespace, unprivileged_client):
     [
         pytest.param(
             {"sockets": 2, "cores": 2, "threads": 2},
-            marks=(pytest.mark.polarion("CNV-2820"), pytest.mark.gating, pytest.mark.conformance, pytest.mark.x86_64()),
+            marks=(pytest.mark.polarion("CNV-2820"), pytest.mark.gating, pytest.mark.conformance),
             id="case1: 2 cores, 2 threads, 2 sockets",
         ),
         pytest.param(
             {"sockets": None, "cores": 1, "threads": 2},
-            marks=(pytest.mark.polarion("CNV-2823"), pytest.mark.x86_64()),
+            marks=(pytest.mark.polarion("CNV-2823")),
             id="case2: 1 cores, 2 threads, no sockets",
         ),
-        pytest.param(
-            {"sockets": 2, "cores": 2, "threads": 1},
-            marks=(pytest.mark.gating(), pytest.mark.s390x()),
-            id="case1: 2 cores, 1 threads, 2 sockets",
-        ),
-        pytest.param(
-            {"sockets": None, "cores": 1, "threads": 1},
-            marks=[pytest.mark.s390x()],
-            id="case2: 1 cores, 1 threads, no sockets",
-        ),
         pytest.param(
             {"sockets": 2, "cores": 1, "threads": None},
-            marks=(pytest.mark.polarion("CNV-2822"), pytest.mark.x86_64(), pytest.mark.s390x()),
+            marks=(pytest.mark.polarion("CNV-2822")),
             id="case3: 1 cores, no threads, 2 sockets",
         ),
         pytest.param(
             {"sockets": None, "cores": 2, "threads": None},
-            marks=(pytest.mark.polarion("CNV-2821"), pytest.mark.x86_64(), pytest.mark.s390x()),
+            marks=(pytest.mark.polarion("CNV-2821")),
             id="case4: 2 cores, no threads, no sockets",
         ),
     ],
     indirect=True,
 )
+@pytest.mark.s390x
 def test_vm_with_cpu_support(vm_with_cpu_support):
     """
     Test VM with cpu support
@@ -107,7 +98,6 @@ def no_cpu_settings_vm(namespace, unprivileged_client):
 
 @pytest.mark.gating
 @pytest.mark.conformance
-@pytest.mark.x86_64
 @pytest.mark.s390x
 @pytest.mark.polarion("CNV-1485")
 def test_vm_with_no_cpu_settings(no_cpu_settings_vm):
@@ -120,7 +110,6 @@ def test_vm_with_no_cpu_settings(no_cpu_settings_vm):
 
 
 @pytest.mark.gating
-@pytest.mark.x86_64
 @pytest.mark.s390x
 @pytest.mark.polarion("CNV-2818")
 def test_vm_with_cpu_limitation(namespace, unprivileged_client):
@@ -144,7 +133,6 @@ def test_vm_with_cpu_limitation(namespace, unprivileged_client):
 
 
 @pytest.mark.polarion("CNV-2819")
-@pytest.mark.x86_64
 @pytest.mark.s390x
 def test_vm_with_cpu_limitation_negative(namespace, unprivileged_client):
     """

tests/virt/node/efi_secureboot/test_vm_with_efi_secureboot.py

@@ -14,7 +14,6 @@
 
 @pytest.mark.gating
 @pytest.mark.polarion("CNV-4465")
-@pytest.mark.x86_64
 def test_efi_secureboot_with_smm_disabled(namespace, unprivileged_client):
     """Test that EFI secureBoot VM with SMM disabled, does not get created"""
     with pytest.raises(UnprocessibleEntityError):

tests/virt/node/general/test_custom_selinux_policy.py

@@ -3,7 +3,6 @@
 from utilities.infra import ExecCommandOnPod
 
 
-@pytest.mark.x86_64
 @pytest.mark.s390x
 @pytest.mark.polarion("CNV-9918")
 def test_customselinuxpolicy(workers_utility_pods, schedulable_nodes):

tests/virt/node/general/test_disable_pvspinlock.py

@@ -20,7 +20,6 @@ def vm_for_test_pvspinlock(
 
 
 @pytest.mark.polarion("CNV-6877")
-@pytest.mark.x86_64
 def test_disable_pvspinlock(vm_for_test_pvspinlock):
     assert vm_for_test_pvspinlock.privileged_vmi.xml_dict["domain"]["features"]["pvspinlock"]["@state"] == "off", (
         "pvspinlock is not disabled in domain xml."

tests/virt/node/general/test_disk_io_option.py

@@ -67,7 +67,6 @@ def disk_options_vm(
 
 @pytest.mark.gating
 @pytest.mark.s390x
-@pytest.mark.x86_64
 @pytest.mark.parametrize(
     "golden_image_data_volume_scope_class,",
     [
@@ -116,7 +115,6 @@ def test_vm_with_disk_io_option_rhel(
 
 
 @pytest.mark.tier3
-@pytest.mark.x86_64
 @pytest.mark.parametrize(
     "golden_image_data_volume_scope_class,",
     [

tests/virt/node/general/test_legacy_machinetype.py

@@ -87,7 +87,7 @@ def rhel_8_10_vm(unprivileged_client, namespace, rhel_8_10_ds):
         )
     ],
 )
-@pytest.mark.x86_64  # no legacy machinetype for s390x which is different from emulated machine types
+# For s390x, there is no legacy machinetype which is different from emulated machine types
 def test_legacy_machine_type(
     updated_hco_emulated_machine_i440fx,
     rhel_8_10_vm,

tests/virt/node/general/test_linux_label.py

@@ -17,7 +17,6 @@ def assert_linux_label_was_added_in_nodes(nodes):
 @pytest.mark.gating
 @pytest.mark.conformance
 @pytest.mark.s390x
-@pytest.mark.x86_64
 @pytest.mark.polarion("CNV-5758")
 def test_linux_label_was_added(schedulable_nodes):
     assert_linux_label_was_added_in_nodes(nodes=schedulable_nodes)