Repository: aristanetworks/anta (python)

Issue:
When using `anta nrfu json --output results.json` we shouldn't have the results on the console

Pull request:
fix(anta): Remove JSON output when saving to a file
# Description

When using the CLI command `anta nrfu json --output ./results.json`, results will be saved in a JSON file without printing to the console.

Fixes #760 

# Checklist:

<!-- Delete not relevant items !-->

- [X] My code follows the style guidelines of this project
- [X] I have performed a self-review of my code
- [X] I have run pre-commit for code linting and typing (`pre-commit run`)
- [X] I have made corresponding changes to the documentation
- [X] I have added tests that prove my fix is effective or that my feature works
- [X] New and existing unit tests pass locally with my changes (`tox -e testenv`)


Patch:
diff --git a/anta/catalog.py b/anta/catalog.py
index 30bd3406..7ed4bc71 100644
--- a/anta/catalog.py
+++ b/anta/catalog.py
@@ -10,9 +10,11 @@ import logging
 import math
 from collections import defaultdict
 from inspect import isclass
+from itertools import chain
 from json import load as json_load
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Literal, Optional, Union
+from warnings import warn
 
 from pydantic import BaseModel, ConfigDict, RootModel, ValidationError, ValidationInfo, field_validator, model_serializer, model_validator
 from pydantic.types import ImportString
@@ -386,6 +388,21 @@ class AntaCatalog:
             raise
         return AntaCatalog(tests)
 
+    @classmethod
+    def merge_catalogs(cls, catalogs: list[AntaCatalog]) -> AntaCatalog:
+        """Merge multiple AntaCatalog instances.
+
+        Parameters
+        ----------
+            catalogs: A list of AntaCatalog instances to merge.
+
+        Returns
+        -------
+            A new AntaCatalog instance containing the tests of all the input catalogs.
+        """
+        combined_tests = list(chain(*(catalog.tests for catalog in catalogs)))
+        return cls(tests=combined_tests)
+
     def merge(self, catalog: AntaCatalog) -> AntaCatalog:
         """Merge two AntaCatalog instances.
 
@@ -397,7 +414,13 @@ class AntaCatalog:
         -------
             A new AntaCatalog instance containing the tests of the two instances.
         """
-        return AntaCatalog(tests=self.tests + catalog.tests)
+        # TODO: Use a decorator to deprecate this method instead. See https://github.com/aristanetworks/anta/issues/754
+        warn(
+            message="AntaCatalog.merge() is deprecated and will be removed in ANTA v2.0. Use AntaCatalog.merge_catalogs() instead.",
+            category=DeprecationWarning,
+            stacklevel=2,
+        )
+        return self.merge_catalogs([self, catalog])
 
     def dump(self) -> AntaCatalogFile:
         """Return an AntaCatalogFile instance from this AntaCatalog instance.
diff --git a/anta/cli/nrfu/commands.py b/anta/cli/nrfu/commands.py
index cd750cb8..6043dbef 100644
--- a/anta/cli/nrfu/commands.py
+++ b/anta/cli/nrfu/commands.py
@@ -42,7 +42,7 @@ def table(ctx: click.Context, group_by: Literal["device", "test"] | None) -> Non
     type=click.Path(file_okay=True, dir_okay=False, exists=False, writable=True, path_type=pathlib.Path),
     show_envvar=True,
     required=False,
-    help="Path to save report as a file",
+    help="Path to save report as a JSON file",
 )
 def json(ctx: click.Context, output: pathlib.Path | None) -> None:
     """ANTA command to check network state with JSON result."""
diff --git a/anta/cli/nrfu/utils.py b/anta/cli/nrfu/utils.py
index 284c9b70..cfc2e1ed 100644
--- a/anta/cli/nrfu/utils.py
+++ b/anta/cli/nrfu/utils.py
@@ -94,14 +94,21 @@ def print_table(ctx: click.Context, group_by: Literal["device", "test"] | None =
 
 
 def print_json(ctx: click.Context, output: pathlib.Path | None = None) -> None:
-    """Print result in a json format."""
+    """Print results as JSON. If output is provided, save to file instead."""
     results = _get_result_manager(ctx)
-    console.print()
-    console.print(Panel("JSON results", style="cyan"))
-    rich.print_json(results.json)
-    if output is not None:
-        with output.open(mode="w", encoding="utf-8") as fout:
-            fout.write(results.json)
+
+    if output is None:
+        console.print()
+        console.print(Panel("JSON results", style="cyan"))
+        rich.print_json(results.json)
+    else:
+        try:
+            with output.open(mode="w", encoding="utf-8") as file:
+                file.write(results.json)
+            console.print(f"JSON results saved to {output} ✅", style="cyan")
+        except OSError:
+            console.print(f"Failed to save JSON results to {output} ❌", style="cyan")
+            ctx.exit(ExitCode.USAGE_ERROR)
 
 
 def print_text(ctx: click.Context) -> None:
diff --git a/docs/cli/nrfu.md b/docs/cli/nrfu.md
index afed2594..2f4e7eed 100644
--- a/docs/cli/nrfu.md
+++ b/docs/cli/nrfu.md
@@ -120,7 +120,7 @@ anta nrfu --test VerifyZeroTouch table
 
 ## Performing NRFU with JSON rendering
 
-The JSON rendering command in NRFU testing is useful in generating a JSON output that can subsequently be passed on to another tool for reporting purposes.
+The JSON rendering command in NRFU testing will generate an output of all test results in JSON format.
 
 ### Command overview
 
@@ -131,12 +131,12 @@ Usage: anta nrfu json [OPTIONS]
   ANTA command to check network state with JSON result.
 
 Options:
-  -o, --output FILE  Path to save report as a file  [env var:
+  -o, --output FILE  Path to save report as a JSON file  [env var:
                      ANTA_NRFU_JSON_OUTPUT]
   --help             Show this message and exit.
 ```
 
-The `--output` option allows you to save the JSON report as a file.
+The `--output` option allows you to save the JSON report as a file. If specified, no output will be displayed in the terminal. This is useful for further processing or integration with other tools.
 
 ### Example
 
diff --git a/docs/usage-inventory-catalog.md b/docs/usage-inventory-catalog.md
index fd6aec32..5ae4cc92 100644
--- a/docs/usage-inventory-catalog.md
+++ b/docs/usage-inventory-catalog.md
@@ -309,7 +309,7 @@ Once you run `anta nrfu table`, you will see following output:
 
 ### Example script to merge catalogs
 
-The following script reads all the files in `intended/test_catalogs/` with names `<device_name>-catalog.yml` and merge them together inside one big catalog `anta-catalog.yml`.
+The following script reads all the files in `intended/test_catalogs/` with names `<device_name>-catalog.yml` and merge them together inside one big catalog `anta-catalog.yml` using the new `AntaCatalog.merge_catalogs()` class method.
 
 ```python
 #!/usr/bin/env python
@@ -319,19 +319,26 @@ from pathlib import Path
 from anta.models import AntaTest
 
 
-CATALOG_SUFFIX = '-catalog.yml'
-CATALOG_DIR = 'intended/test_catalogs/'
+CATALOG_SUFFIX = "-catalog.yml"
+CATALOG_DIR = "intended/test_catalogs/"
 
 if __name__ == "__main__":
-    catalog = AntaCatalog()
-    for file in Path(CATALOG_DIR).glob('*'+CATALOG_SUFFIX):
-        c = AntaCatalog.parse(file)
+    catalogs = []
+    for file in Path(CATALOG_DIR).glob("*" + CATALOG_SUFFIX):
         device = str(file).removesuffix(CATALOG_SUFFIX).removeprefix(CATALOG_DIR)
-        print(f"Merging test catalog for device {device}")
-        # Apply filters to all tests for this device
-        for test in c.tests:
-            test.inputs.filters = AntaTest.Input.Filters(tags=[device])
-        catalog = catalog.merge(c)
+        print(f"Loading test catalog for device {device}")
+        catalog = AntaCatalog.parse(file)
+        # Add the device name as a tag to all tests in the catalog
+        for test in catalog.tests:
+            test.inputs.filters = AntaTest.Input.Filters(tags={device})
+        catalogs.append(catalog)
+
+    # Merge all catalogs
+    merged_catalog = AntaCatalog.merge_catalogs(catalogs)
+
+    # Save the merged catalog to a file
     with open(Path('anta-catalog.yml'), "w") as f:
         f.write(catalog.dump().yaml())
 ```
+!!! warning
+    The `AntaCatalog.merge()` method is deprecated and will be removed in ANTA v2.0. Please use the `AntaCatalog.merge_catalogs()` class method instead.


Test patch:
diff --git a/tests/units/cli/nrfu/test_commands.py b/tests/units/cli/nrfu/test_commands.py
index 8ad7745f..803c8f80 100644
--- a/tests/units/cli/nrfu/test_commands.py
+++ b/tests/units/cli/nrfu/test_commands.py
@@ -8,7 +8,7 @@ from __future__ import annotations
 import json
 import re
 from pathlib import Path
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Any
 from unittest.mock import patch
 
 from anta.cli import anta
@@ -90,6 +90,43 @@ def test_anta_nrfu_json(click_runner: CliRunner) -> None:
             assert res["result"] == "success"
 
 
+def test_anta_nrfu_json_output(click_runner: CliRunner, tmp_path: Path) -> None:
+    """Test anta nrfu json with output file."""
+    json_output = tmp_path / "test.json"
+    result = click_runner.invoke(anta, ["nrfu", "json", "--output", str(json_output)])
+
+    # Making sure the output is not printed to stdout
+    match = re.search(r"\[\n {2}{[\s\S]+ {2}}\n\]", result.output)
+    assert match is None
+
+    assert result.exit_code == ExitCode.OK
+    assert "JSON results saved to" in result.output
+    assert json_output.exists()
+
+
+def test_anta_nrfu_json_output_failure(click_runner: CliRunner, tmp_path: Path) -> None:
+    """Test anta nrfu json with output file."""
+    json_output = tmp_path / "test.json"
+
+    original_open = Path.open
+
+    def mock_path_open(*args: Any, **kwargs: Any) -> Path:  # noqa: ANN401
+        """Mock Path.open only for the json_output file of this test."""
+        if args[0] == json_output:
+            msg = "Simulated OSError"
+            raise OSError(msg)
+
+        # If not the json_output file, call the original Path.open
+        return original_open(*args, **kwargs)
+
+    with patch("pathlib.Path.open", mock_path_open):
+        result = click_runner.invoke(anta, ["nrfu", "json", "--output", str(json_output)])
+
+    assert result.exit_code == ExitCode.USAGE_ERROR
+    assert "Failed to save JSON results to" in result.output
+    assert not json_output.exists()
+
+
 def test_anta_nrfu_template(click_runner: CliRunner) -> None:
     """Test anta nrfu, catalog is given via env."""
     result = click_runner.invoke(anta, ["nrfu", "tpl-report", "--template", str(DATA_DIR / "template.j2")])
diff --git a/tests/units/test_catalog.py b/tests/units/test_catalog.py
index 76358dd4..13046f29 100644
--- a/tests/units/test_catalog.py
+++ b/tests/units/test_catalog.py
@@ -345,6 +345,17 @@ class TestAntaCatalog:
         tests = catalog.get_tests_by_tags(tags={"leaf", "spine"}, strict=True)
         assert len(tests) == 1
 
+    def test_merge_catalogs(self) -> None:
+        """Test the merge_catalogs function."""
+        # Load catalogs of different sizes
+        small_catalog = AntaCatalog.parse(DATA_DIR / "test_catalog.yml")
+        medium_catalog = AntaCatalog.parse(DATA_DIR / "test_catalog_medium.yml")
+        tagged_catalog = AntaCatalog.parse(DATA_DIR / "test_catalog_with_tags.yml")
+
+        # Merge the catalogs and check the number of tests
+        final_catalog = AntaCatalog.merge_catalogs([small_catalog, medium_catalog, tagged_catalog])
+        assert len(final_catalog.tests) == len(small_catalog.tests) + len(medium_catalog.tests) + len(tagged_catalog.tests)
+
     def test_merge(self) -> None:
         """Test AntaCatalog.merge()."""
         catalog1: AntaCatalog = AntaCatalog.parse(DATA_DIR / "test_catalog.yml")
@@ -354,11 +365,15 @@ class TestAntaCatalog:
         catalog3: AntaCatalog = AntaCatalog.parse(DATA_DIR / "test_catalog_medium.yml")
         assert len(catalog3.tests) == 228
 
-        assert len(catalog1.merge(catalog2).tests) == 2
+        with pytest.deprecated_call():
+            merged_catalog = catalog1.merge(catalog2)
+        assert len(merged_catalog.tests) == 2
         assert len(catalog1.tests) == 1
         assert len(catalog2.tests) == 1
 
-        assert len(catalog2.merge(catalog3).tests) == 229
+        with pytest.deprecated_call():
+            merged_catalog = catalog2.merge(catalog3)
+        assert len(merged_catalog.tests) == 229
         assert len(catalog2.tests) == 1
         assert len(catalog3.tests) == 228