Address code review comments

Simplify unit test for generation of Slurm accounting configuration files. Remove commented parameters in slurm kitchen test configuration file. Modify copyright year in one file. Signed-off-by: Jacopo De Amicis <[email protected]>
aws · Sep 27, 2023 · b401a23 · b401a23
1 parent 4e34fee
commit b401a23
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 18 deletions.
diff --git a/cookbooks/aws-parallelcluster-slurm/kitchen.slurm-config.yml b/cookbooks/aws-parallelcluster-slurm/kitchen.slurm-config.yml
@@ -207,9 +207,6 @@ suites:
       cluster:
         node_type: HeadNode
         scheduler: 'slurm'
-        #slurm:
-        #  user: 'slurm'
-        #  group: 'slurm'
         config:
           Scheduling:
             SlurmSettings:

diff --git a/cookbooks/aws-parallelcluster-slurm/recipes/install/slurm_users.rb b/cookbooks/aws-parallelcluster-slurm/recipes/install/slurm_users.rb
@@ -4,7 +4,7 @@
 # Cookbook:: aws-parallelcluster-slurm
 # Recipe:: install_slurm
 #
-# Copyright:: 2013-2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+# Copyright:: 2013-2023 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License"). You may not use this file except in compliance with the
 # License. A copy of the License is located at

diff --git a/test/unit/slurm/test_slurm_config_generator.py b/test/unit/slurm/test_slurm_config_generator.py
@@ -102,17 +102,36 @@ def test_generate_slurm_config_files_memory_scheduling(
 
 
 @pytest.mark.parametrize(
-    "slurm_accounting, dbname_passed",
-    [(False, False), (True, False), (True, True)],
+    "input_config, expected_outputs",
+    [
+        pytest.param(
+            "sample_input.yaml",
+            [
+                "slurm_parallelcluster.conf",
+                "slurm_parallelcluster_slurmdbd.conf",
+            ],
+            id="Case without Slurm Accounting",
+        ),
+        pytest.param(
+            "sample_input_slurm_accounting.yaml",
+            [
+                "slurm_parallelcluster_slurm_accounting.conf",
+                "slurm_parallelcluster_slurmdbd_slurm_accounting.conf",
+            ],
+            id="Case with Slurm Accounting",
+        ),
+        pytest.param(
+            "sample_input_slurm_accounting_dbname.yaml",
+            [
+                "slurm_parallelcluster_slurm_accounting_dbname.conf",
+                "slurm_parallelcluster_slurmdbd_slurm_accounting_dbname.conf",
+            ],
+            id="Case with Slurm Accounting passing DatabaseName",
+        ),
+    ],
 )
-def test_generate_slurm_config_files_slurm_accounting(mocker, test_datadir, tmpdir, slurm_accounting, dbname_passed):
-    input_base_name = "sample_input"
-    postfix = ""
-    if slurm_accounting:
-        postfix += "_slurm_accounting"
-    if dbname_passed:
-        postfix += "_dbname"
-    input_file = str(test_datadir / (input_base_name + postfix + ".yaml"))
+def test_generate_slurm_config_files_slurm_accounting(mocker, test_datadir, tmpdir, input_config, expected_outputs):
+    input_file = str(test_datadir / input_config)
     instance_types_data = str(test_datadir / "sample_instance_types_data.json")
 
     _mock_head_node_config(mocker)
@@ -130,10 +149,12 @@ def test_generate_slurm_config_files_slurm_accounting(mocker, test_datadir, tmpd
         cluster_name="test-cluster",
     )
 
-    for file_type in ["", "_slurmdbd"]:
-        file_name = f"slurm_parallelcluster{file_type}.conf"
-        output_file_name = f"slurm_parallelcluster{file_type}{postfix}.conf"
-        _assert_files_are_equal(tmpdir / file_name, test_datadir / "expected_outputs" / output_file_name)
+    generated_outputs = ["slurm_parallelcluster.conf", "slurm_parallelcluster_slurmdbd.conf"]
+
+    for item in zip(generated_outputs, expected_outputs):
+        generated_file = str(tmpdir / item[0])
+        expected_file = str(test_datadir / "expected_outputs" / item[1])
+        _assert_files_are_equal(generated_file, expected_file)
 
 
 def test_generating_slurm_config_flexible_instance_types(mocker, test_datadir, tmpdir):