Fix: Make CI/CD tests work with Ruamel

docs/dev/tests.md

@@ -5,10 +5,11 @@
 * **mypy** static type checking of all Python code in `netsim/` directory
 * Transformation tests ran with **pytest** in `tests/` directory
 
-You can run the same tests with the `run-tests.sh` script in *tests* directory. It's highly recommended you run them before creating a pull request that modifies Python code, or we'll have to have a discussion before your PR is merged.
+You can run the same tests with the `run-tests.sh` script in *tests* directory. It's highly recommended that you run them before creating a pull request that modifies Python code. PRs that fail the CI/CD pipeline will not be merged.
 
 ```{tip}
-The CI/CD tests require additional Python modules. Install them with `pip3 install -r requirements-dev.txt`
+* The CI/CD tests require additional Python modules. Install them with `pip3 install -r requirements-dev.txt`
+* The CI/CD tests use PyYAML. You can run them on a system with `ruamel.yaml` installed, but they might take longer (see [bug report](https://github.com/ipspace/netlab/issues/3345) and [related PR](https://github.com/ipspace/netlab/pull/3353) for details). Uninstalling `ruamel.yaml` might not be a bad idea. 
 ```
 
 ## Automated Tests
@@ -43,7 +44,11 @@ The transformation error tests:
 
 Whenever you're creating a new error test case or modifying an existing one, you **HAVE TO** create a corresponding *expected error messages* log file.
 
-To create the *expected error messages* files run `create-error-tests.sh` script in the *tests* directory. The script assumes that your code works flawlessly and that whatever error messages are generated are the expected error messages. That might *not* be the case, so it's highly recommended that you execute `git diff errors` after running `create-errors-tests.sh` script and do a thorough check of the differences.
+To create the *expected error messages* files, run the `create-error-tests.sh` script in the *tests* directory. The script assumes that your code works flawlessly and that whatever error messages are generated are the expected error messages. That might *not* be the case, so it's highly recommended that you execute `git diff errors` after running the `create-errors-tests.sh` script and do a thorough check of the differences.
+
+```{warning}
+You cannot create a new error test on a system with `ruamel.yaml` package (details in the [bug report](https://github.com/ipspace/netlab/issues/3345) and [related PR](https://github.com/ipspace/netlab/pull/3353)). Uninstall `ruamel.yaml` before running the `create-error-tests.sh` script.
+```
 
 ## Before Submitting a PR
 

tests/conftest.py

@@ -0,0 +1,19 @@
+#
+# Pytest hooks shared across the netlab test tree
+#
+
+import pytest
+from utils import HAS_RUAMEL
+
+
+def pytest_configure(config: pytest.Config) -> None:
+  if not HAS_RUAMEL:
+    return
+  config.issue_config_time_warning(
+    UserWarning(
+      "ruamel.yaml is installed; transformation tests will be slower and "
+      "`create-error-tests.sh` is unsupported "
+      "(see https://github.com/ipspace/netlab/issues/3345)."
+    ),
+    stacklevel=2,
+  )

tests/create-error-tests.sh

@@ -4,6 +4,12 @@
 # on STDERR into a file that will be used in the test harness
 # to validate the error handling hasn't been broken.
 #
+if python3 -c "import ruamel.yaml" 2>/dev/null; then
+  echo "ERROR: ruamel.yaml is installed; error-test fixtures cannot be generated" >&2
+  echo "       correctly. Uninstall ruamel.yaml first (see #3345)." >&2
+  exit 2
+fi
+
 ERR_PATH="errors"
 
 # Check for "coverage" first

tests/create-transformation-test-case.py

@@ -5,22 +5,24 @@
 #
 
 import argparse
+import sys
 
 import utils
+from box import Box
 
 import netsim.augment
-import netsim.common
+from netsim.data import get_box
 from netsim.utils import log
 from netsim.utils import read as _read
 
 
-def create_expected_results_file(topology,fname):
+def create_expected_results_file(topology: Box,fname: str) -> None:
   with open(fname,"w") as output:
     output.write(utils.transformation_results_yaml(topology))
     output.close()
     print(f"... created expected transformed topology: {fname}")
 
-def parse():
+def parse() -> argparse.Namespace:
   parser = argparse.ArgumentParser(description='Create topology test cases')
   parser.add_argument('-t','--topology', dest='topology', action='store', default='topology.yml',
                   help='Topology file name')
@@ -31,10 +33,18 @@ def parse():
 
   return args
 
-def main():
+def main() -> None:
   args = parse()
+  if utils.HAS_RUAMEL:
+    print(
+      "WARNING: ruamel.yaml is installed; fixture generation will be slower. "
+      "Consider uninstalling ruamel.yaml (see #3345).",
+      file=sys.stderr,
+    )
   print(f"Reading {args.topology}")
   topology = _read.load(args.topology,user_defaults=[],relative_topo_name=True)
+  if utils.HAS_RUAMEL:
+    topology = get_box(utils.clean_ruamel(topology))
   log.exit_on_error()
   netsim.augment.main.transform(topology)
   log.exit_on_error()

tests/test_transformation.py

@@ -14,20 +14,23 @@
 from box import Box
 
 from netsim import augment
+from netsim.data import get_box
 from netsim.outputs import _TopologyOutput, ansible
 from netsim.utils import log
 from netsim.utils import read as _read
 
 
-def run_test(fname):
+def run_test(fname: str) -> Box:
   log.init_log_system(header = False)
   topology = _read.load(fname,relative_topo_name=True,user_defaults=[])
+  if utils.HAS_RUAMEL:
+    topology = get_box(utils.clean_ruamel(topology))
   log.exit_on_error()
   augment.main.transform(topology)
   log.exit_on_error()
   return topology
 
-def run_transformation_test(test_case: str, tmpdir: str = '/tmp'):
+def run_transformation_test(test_case: str, tmpdir: str = '/tmp') -> None:
   print("Test case: %s" % test_case)
   log.set_flag(raise_error = False)
   topology = run_test(test_case)
@@ -62,14 +65,14 @@ def run_transformation_test(test_case: str, tmpdir: str = '/tmp'):
   print("... succeeded, string length = %d" % len(result))
 
 @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning")
-def test_xform_cases(tmpdir):
+def test_xform_cases(tmpdir: str) -> None:
   print("Starting transformation test cases")
   for test_case in list(glob.glob('topology/input/*yml')):
     run_transformation_test(test_case)
 
 # Verbose test cases are executed only when we're doing a coverage report
 #
-def test_coverage_verbose_cases(tmpdir):
+def test_coverage_verbose_cases(tmpdir: str) -> None:
   if not sys.gettrace():
     return
   log.set_verbose()
@@ -78,7 +81,6 @@ def test_coverage_verbose_cases(tmpdir):
 def run_error_case(test_case: str) -> None:
   log.set_flag(raise_error = True)
   print("Test case: %s" % test_case)
-  log.err_count = 0
   with pytest.raises(log.ErrorAbort):
     run_test(test_case)
 
@@ -95,18 +97,18 @@ def run_error_case(test_case: str) -> None:
     assert error_log == log_lines
 
 @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning")
-def test_error_cases():
+def test_error_cases() -> None:
   print("Starting error test cases")
   for test_case in list(glob.glob('errors/*yml')):
     run_error_case(test_case)
 
 @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning")
-def test_coverage_xf_cases():
+def test_coverage_xf_cases() -> None:
   for test_case in list(glob.glob('coverage/input/*yml')):
     run_transformation_test(test_case)
 
 @pytest.mark.filterwarnings("ignore::PendingDeprecationWarning")
-def test_coverage_errors():
+def test_coverage_errors() -> None:
   for test_case in list(glob.glob('coverage/errors/*yml')):
     run_error_case(test_case)
 

tests/utils.py

@@ -2,10 +2,43 @@
 # Test utilities
 #
 
+import typing
+
+import yaml
 from box import Box
 
 from netsim.augment.nodes import ghost_buster
 
+try:
+  import ruamel.yaml  # type: ignore # noqa: F401 -- detect the actual library, not just the namespace shim
+  HAS_RUAMEL = True
+except ImportError:
+  HAS_RUAMEL = False
+
+def clean_ruamel(data: typing.Any) -> typing.Any:
+  """
+  Ruamel loves to use data types derived from int/str/float. If we want to
+  create YAML via PyYAML (see #3345), we need to go through the whole data
+  structure and clean it up. Yeah, it will be slow. Hooray for consistency!
+  """
+  if not HAS_RUAMEL:
+    return data
+
+  if isinstance(data,bool):
+    return data
+  elif isinstance(data,int):
+    return int(data)
+  elif isinstance(data,str):
+    return str(data)
+  elif isinstance(data,float):
+    return float(data)
+  elif isinstance(data,list):
+    return [ clean_ruamel(v) for v in data ]
+  elif isinstance(data,dict):
+    return { k:clean_ruamel(v) for k,v in data.items() }
+  else:
+    return data
+
 """
 Return the results of topology transformation YAML format
 
@@ -20,4 +53,4 @@ def transformation_results_yaml(topology: Box) -> str:
   if 'unmanaged' in ignore:
     topology = ghost_buster(topology)
 
-  return topology.to_yaml()
+  return yaml.dump(topology.to_dict(),default_flow_style=False,width=120)