Merge pull request #560 from projectsyn/fix/package-new-update-test-c…

…ases

Deduplicate requested test cases for `package new` and `package update`

commodore/cli.py

@@ -483,7 +483,8 @@ def package(config: Config, verbose: int):
     show_default=True,
     multiple=True,
     help="Additional test cases to generate in the new package. Can be repeated. "
-    + "Test case `defaults` will always be generated.",
+    + "Test case `defaults` will always be generated."
+    + "Commodore will deduplicate test cases by name.",
 )
 @verbosity
 @pass_config
@@ -550,7 +551,8 @@ def package_new(
     show_default=True,
     multiple=True,
     help="Additional test cases to generate in the new package. Can be repeated. "
-    + "Already existing test cases will always be kept.",
+    + "Already existing test cases will always be kept. "
+    + "Commodore will deduplicate test cases by name.",
 )
 @click.option(
     "--remove-test-case",
@@ -583,8 +585,9 @@ def package_update(
         t.golden_tests = golden_tests
     if update_copyright_year:
         t.copyright_year = None
-    t.test_cases.extend(additional_test_case)
-    t.test_cases = [tc for tc in t.test_cases if tc not in remove_test_case]
+    test_cases = t.test_cases
+    test_cases.extend(additional_test_case)
+    t.test_cases = [tc for tc in test_cases if tc not in remove_test_case]
 
     t.update()
 

commodore/package/template.py

@@ -23,7 +23,7 @@ class PackageTemplater(Templater):
     template_url: str
     template_version: str
     template_commit: str
-    test_cases: list[str] = ["defaults"]
+    _test_cases: list[str] = ["defaults"]
     copyright_year: Optional[str] = None
     _target_dir: Optional[Path] = None
 
@@ -54,6 +54,24 @@ def from_existing(cls, config: Config, package_path: Path):
         t.copyright_year = cookiecutter_args["copyright_year"]
         return t
 
+    @property
+    def test_cases(self) -> list[str]:
+        """Return list of test cases.
+
+        The getter deduplicates the stored list before returning it.
+
+        Don't use `append()` on the returned list to add test cases to the package, as
+        the getter returns a copy of the list stored in the object."""
+        cases = []
+        for t in self._test_cases:
+            if t not in cases:
+                cases.append(t)
+        return cases
+
+    @test_cases.setter
+    def test_cases(self, test_cases: list[str]):
+        self._test_cases = test_cases
+
     def _cruft_renderer(
         self,
         template_location: str,

docs/modules/ROOT/pages/reference/cli.adoc

@@ -234,6 +234,7 @@ This command doesn't have any command line options.
   Additional test cases to generate in the new package.
   Can be repeated.
   Test case `defaults` will always be generated.
+  Commodore will deduplicate the provided test cases.
 
 == Package Update
 
@@ -248,6 +249,7 @@ This command doesn't have any command line options.
 *--additional-test-case, -t* CASE::
   Additional test cases to add to the package.
   Can be repeated.
+  Commodore will deduplicate the provided test cases.
 
 *--remove-test-case* CASE::
   Test cases to remove from the package.

tests/test_package_template.py

@@ -57,6 +57,7 @@ def call_package_new(
     [
         [],
         ["foo"],
+        ["foo", "foo"],
         ["foo", "bar"],
     ],
 )
@@ -103,10 +104,15 @@ def test_run_package_new_command(
     for f in expected_files:
         assert (pkg_dir / f).is_file()
 
+    expected_cases = ["defaults"]
+    for t in additional_test_cases:
+        if t not in expected_cases:
+            expected_cases.append(t)
+
     with open(pkg_dir / ".github" / "workflows" / "test.yaml") as gh_test:
         workflows = yaml.safe_load(gh_test)
         instances = workflows["jobs"]["test"]["strategy"]["matrix"]["instance"]
-        assert instances == list(["defaults"] + additional_test_cases)
+        assert instances == expected_cases
 
 
 @pytest.mark.parametrize(
@@ -293,3 +299,23 @@ def test_package_templater_from_existing_nonexistent(tmp_path: Path, config: Con
         _ = PackageTemplater.from_existing(config, tmp_path / "test-package")
 
     assert str(e.value) == "Provided package path isn't a directory"
+
+
+@pytest.mark.parametrize(
+    "test_cases,expected",
+    [
+        ([], []),
+        (["defaults"], ["defaults"]),
+        (["defaults", "foo"], ["defaults", "foo"]),
+        (["defaults", "foo", "foo"], ["defaults", "foo"]),
+        (["foo", "bar"], ["foo", "bar"]),
+        (["foo", "bar", "foo"], ["foo", "bar"]),
+    ],
+)
+def test_package_templater_test_cases(
+    tmp_path: Path, config: Config, test_cases: list[str], expected: list[str]
+):
+    p = PackageTemplater(config, "test-package")
+    p.test_cases = test_cases
+
+    assert p.test_cases == expected