Ensure tests use ncgen/ncdump from the Python env; rationalise uses o…

…f ncgen. (#4794)

* Ensure tests use ncgen/ncdump from the Python env; rationalise uses of ncgen a bit.

* Tidy some imports.

* Rationalise use of ncgen and avoid subprocess 'shell=True'.

* Review changes: revert to importing iris.tests first.

* Tidy routines slightly.

* Fix subprocess call.

* Added whatsnew.

docs/src/whatsnew/latest.rst

@@ -216,6 +216,12 @@ This document explains the changes made to Iris for this release
 #. `@bjlittle`_ migrated to GitHub Actions for Continuous-Integration.
    (:pull:`4503`)
 
+#. `@pp-mo`_ made tests run certain linux executables from the Python env,
+   specifically ncdump and ncgen.  These could otherwise fail when run in IDEs
+   such as PyCharm and Eclipse, which don't automatically include the Python env
+   bin in the system PATH.
+   (:pull:`4794`)
+
 
 .. comment
     Whatsnew author names (@github name) in alphabetical order. Note that,

lib/iris/tests/__init__.py

@@ -34,12 +34,13 @@
 import math
 import os
 import os.path
+from pathlib import Path
 import re
 import shutil
 import subprocess
 import sys
 import threading
-from typing import Dict, List
+from typing import AnyStr, Dict, List
 import unittest
 from unittest import mock
 import warnings
@@ -373,8 +374,8 @@ def assertCDL(self, netcdf_filename, reference_filename=None, flags="-h"):
             flags = list(map(str, flags))
 
         try:
-            # Python3 only: use subprocess.run()
-            args = ["ncdump"] + flags + [netcdf_filename]
+            exe_path = env_bin_path("ncdump")
+            args = [exe_path] + flags + [netcdf_filename]
             cdl = subprocess.check_output(args, stderr=subprocess.STDOUT)
         except subprocess.CalledProcessError as exc:
             print(exc.output)
@@ -1348,3 +1349,30 @@ def wrapped(self, *args, **kwargs):
         return result
 
     return wrapped
+
+
+def env_bin_path(exe_name: AnyStr = None):
+    """
+    Return a Path object for (an executable in) the environment bin directory.
+
+    Parameters
+    ----------
+    exe_name : str
+        If set, the name of an executable to append to the path.
+
+    Returns
+    -------
+    exe_path : Path
+        A path to the bin directory, or an executable file within it.
+
+    Notes
+    -----
+    For use in tests which spawn commands which should call executables within
+    the Python environment, since many IDEs (Eclipse, PyCharm) don't
+    automatically include this location in $PATH (as opposed to $PYTHONPATH).
+    """
+    exe_path = Path(os.__file__)
+    exe_path = (exe_path / "../../../bin").resolve()
+    if exe_name is not None:
+        exe_path = exe_path / exe_name
+    return exe_path

lib/iris/tests/integration/experimental/test_ugrid_save.py

@@ -14,17 +14,15 @@
 import glob
 from pathlib import Path
 import shutil
-from subprocess import check_call
 import tempfile
 
 import iris
 from iris.experimental.ugrid.load import PARSE_UGRID_ON_LOAD
 import iris.fileformats.netcdf
-from iris.tests import IrisTest
-from iris.tests.stock.netcdf import _add_standard_data
+from iris.tests.stock.netcdf import _add_standard_data, ncgen_from_cdl
 
 
-class TestBasicSave(IrisTest):
+class TestBasicSave(tests.IrisTest):
     @classmethod
     def setUpClass(cls):
         cls.temp_dir = Path(tempfile.mkdtemp())
@@ -46,11 +44,11 @@ def tearDownClass(cls):
 
     def test_example_result_cdls(self):
         # Snapshot the result of saving the example cases.
-        for ex_name, filepath in self.example_names_paths.items():
+        for ex_name, cdl_path in self.example_names_paths.items():
+            # Create a test netcdf file.
             target_ncfile_path = str(self.temp_dir / f"{ex_name}.nc")
-            # Create a netcdf file from the test CDL.
-            check_call(
-                f"ncgen {filepath} -k4 -o {target_ncfile_path}", shell=True
+            ncgen_from_cdl(
+                cdl_str=None, cdl_path=cdl_path, nc_path=target_ncfile_path
             )
             # Fill in blank data-variables.
             _add_standard_data(target_ncfile_path)
@@ -64,18 +62,18 @@ def test_example_result_cdls(self):
             refdir_relpath = (
                 "integration/experimental/ugrid_save/TestBasicSave/"
             )
-            reffile_name = str(Path(filepath).name).replace(".nc", ".cdl")
+            reffile_name = str(Path(cdl_path).name).replace(".nc", ".cdl")
             reffile_path = refdir_relpath + reffile_name
             self.assertCDL(resave_ncfile_path, reference_filename=reffile_path)
 
     def test_example_roundtrips(self):
         # Check that save-and-loadback leaves Iris data unchanged,
         # for data derived from each UGRID example CDL.
-        for ex_name, filepath in self.example_names_paths.items():
+        for ex_name, cdl_path in self.example_names_paths.items():
+            # Create a test netcdf file.
             target_ncfile_path = str(self.temp_dir / f"{ex_name}.nc")
-            # Create a netcdf file from the test CDL.
-            check_call(
-                f"ncgen {filepath} -k4 -o {target_ncfile_path}", shell=True
+            ncgen_from_cdl(
+                cdl_str=None, cdl_path=cdl_path, nc_path=target_ncfile_path
             )
             # Fill in blank data-variables.
             _add_standard_data(target_ncfile_path)

lib/iris/tests/integration/test_climatology.py

@@ -13,14 +13,14 @@
 from os.path import join as path_join
 from os.path import sep as os_sep
 import shutil
-from subprocess import check_call
 import tempfile
 
 import iris
 from iris.tests import stock
+from iris.tests.stock.netcdf import ncgen_from_cdl
 
 
-class TestClimatology(iris.tests.IrisTest):
+class TestClimatology(tests.IrisTest):
     reference_cdl_path = os_sep.join(
         [
             dirname(tests.__file__),
@@ -58,12 +58,13 @@ def setUpClass(cls):
         cls.temp_dir = tempfile.mkdtemp()
         cls.path_ref_cdl = path_join(cls.temp_dir, "standard.cdl")
         cls.path_ref_nc = path_join(cls.temp_dir, "standard.nc")
-        # Create reference CDL file.
-        with open(cls.path_ref_cdl, "w") as f_out:
-            f_out.write(cls._simple_cdl_string())
-        # Create reference netCDF file from reference CDL.
-        command = "ncgen -o {} {}".format(cls.path_ref_nc, cls.path_ref_cdl)
-        check_call(command, shell=True)
+        # Create reference CDL and netcdf files (with ncgen).
+        ncgen_from_cdl(
+            cdl_str=cls._simple_cdl_string(),
+            cdl_path=cls.path_ref_cdl,
+            nc_path=cls.path_ref_nc,
+        )
+
         cls.path_temp_nc = path_join(cls.temp_dir, "tmp.nc")
 
         # Create reference cube.

lib/iris/tests/integration/test_netcdf.py

@@ -14,7 +14,6 @@
 import os.path
 from os.path import join as path_join
 import shutil
-from subprocess import check_call
 import tempfile
 from unittest import mock
 import warnings
@@ -33,6 +32,7 @@
     UnknownCellMethodWarning,
 )
 import iris.tests.stock as stock
+from iris.tests.stock.netcdf import ncgen_from_cdl
 import iris.tests.unit.fileformats.netcdf.test_load_cubes as tlc
 
 
@@ -864,12 +864,12 @@ def setUpClass(cls):
         cls.temp_dir = tempfile.mkdtemp()
         cls.path_test_cdl = path_join(cls.temp_dir, "geos_problem.cdl")
         cls.path_test_nc = path_join(cls.temp_dir, "geos_problem.nc")
-        # Create a reference file from the CDL text.
-        with open(cls.path_test_cdl, "w") as f_out:
-            f_out.write(cls._geostationary_problem_cdl)
-        # Call 'ncgen' to make an actual netCDF file from the CDL.
-        command = "ncgen -o {} {}".format(cls.path_test_nc, cls.path_test_cdl)
-        check_call(command, shell=True)
+        # Create reference CDL and netcdf files from the CDL text.
+        ncgen_from_cdl(
+            cdl_str=cls._geostationary_problem_cdl,
+            cdl_path=cls.path_test_cdl,
+            nc_path=cls.path_test_nc,
+        )
 
     @classmethod
     def tearDownClass(cls):

lib/iris/tests/stock/netcdf.py

@@ -8,12 +8,60 @@
 from pathlib import Path
 from string import Template
 import subprocess
+from typing import Optional
 
 import dask
 from dask import array as da
 import netCDF4
 import numpy as np
 
+from iris.tests import env_bin_path
+
+NCGEN_PATHSTR = str(env_bin_path("ncgen"))
+
+
+def ncgen_from_cdl(
+    cdl_str: Optional[str], cdl_path: Optional[str], nc_path: str
+):
+    """
+    Generate a test netcdf file from cdl.
+
+    Source is CDL in either a string or a file.
+    If given a string, will either save a CDL file, or pass text directly.
+    A netcdf output file is always created, at the given path.
+
+    Parameters
+    ----------
+    cdl_str : str or None
+        String containing a CDL description of a netcdf file.
+        If None, 'cdl_path' must be an existing file.
+    cdl_path : str or None
+        Path of temporary text file where cdl_str is written.
+        If None, 'cdl_str' must be given, and is piped direct to ncgen.
+    nc_path : str
+        Path of temporary netcdf file where converted result is put.
+
+    Notes
+    -----
+    For legacy reasons, the path args are 'str's not 'Path's.
+
+    """
+    if cdl_str and cdl_path:
+        with open(cdl_path, "w") as f_out:
+            f_out.write(cdl_str)
+    if cdl_path:
+        # Create netcdf from stored CDL file.
+        call_args = [NCGEN_PATHSTR, cdl_path, "-k4", "-o", nc_path]
+        call_kwargs = {}
+    else:
+        # No CDL file : pipe 'cdl_str' directly into the ncgen program.
+        if not cdl_str:
+            raise ValueError("Must provide either 'cdl_str' or 'cdl_path'.")
+        call_args = [NCGEN_PATHSTR, "-k4", "-o", nc_path]
+        call_kwargs = dict(input=cdl_str, encoding="ascii")
+
+    subprocess.run(call_args, check=True, **call_kwargs)
+
 
 def _file_from_cdl_template(
     temp_file_dir, dataset_name, dataset_type, template_subs
@@ -39,12 +87,7 @@ def _file_from_cdl_template(
 
     # Spawn an "ncgen" command to create an actual NetCDF file from the
     # CDL string.
-    subprocess.run(
-        ["ncgen", "-o" + str(nc_write_path)],
-        input=cdl,
-        encoding="ascii",
-        check=True,
-    )
+    ncgen_from_cdl(cdl_str=cdl, cdl_path=None, nc_path=nc_write_path)
 
     return nc_write_path
 

lib/iris/tests/test_netcdf.py

@@ -16,7 +16,6 @@
 import os.path
 import shutil
 import stat
-from subprocess import check_call
 import tempfile
 from unittest import mock
 
@@ -33,6 +32,7 @@
 from iris.fileformats.netcdf import load_cubes as nc_load_cubes
 import iris.std_names
 import iris.tests.stock as stock
+from iris.tests.stock.netcdf import ncgen_from_cdl
 import iris.util
 
 
@@ -363,12 +363,8 @@ def test_um_stash_source(self):
         self.tmpdir = tempfile.mkdtemp()
         cdl_path = os.path.join(self.tmpdir, "tst.cdl")
         nc_path = os.path.join(self.tmpdir, "tst.nc")
-        # Write CDL string into a temporary CDL file.
-        with open(cdl_path, "w") as f_out:
-            f_out.write(ref_cdl)
-        # Use ncgen to convert this into an actual (temporary) netCDF file.
-        command = "ncgen -o {} {}".format(nc_path, cdl_path)
-        check_call(command, shell=True)
+        # Create a temporary netcdf file from the CDL string.
+        ncgen_from_cdl(ref_cdl, cdl_path, nc_path)
         # Load with iris.fileformats.netcdf.load_cubes, and check expected content.
         cubes = list(nc_load_cubes(nc_path))
         self.assertEqual(len(cubes), 1)
@@ -412,12 +408,8 @@ def test_ukmo__um_stash_source_priority(self):
         self.tmpdir = tempfile.mkdtemp()
         cdl_path = os.path.join(self.tmpdir, "tst.cdl")
         nc_path = os.path.join(self.tmpdir, "tst.nc")
-        # Write CDL string into a temporary CDL file.
-        with open(cdl_path, "w") as f_out:
-            f_out.write(ref_cdl)
-        # Use ncgen to convert this into an actual (temporary) netCDF file.
-        command = "ncgen -o {} {}".format(nc_path, cdl_path)
-        check_call(command, shell=True)
+        # Create a temporary netcdf file from the CDL string.
+        ncgen_from_cdl(ref_cdl, cdl_path, nc_path)
         # Load with iris.fileformats.netcdf.load_cubes, and check expected content.
         cubes = list(nc_load_cubes(nc_path))
         self.assertEqual(len(cubes), 1)
@@ -457,12 +449,8 @@ def test_bad_um_stash_source(self):
         self.tmpdir = tempfile.mkdtemp()
         cdl_path = os.path.join(self.tmpdir, "tst.cdl")
         nc_path = os.path.join(self.tmpdir, "tst.nc")
-        # Write CDL string into a temporary CDL file.
-        with open(cdl_path, "w") as f_out:
-            f_out.write(ref_cdl)
-        # Use ncgen to convert this into an actual (temporary) netCDF file.
-        command = "ncgen -o {} {}".format(nc_path, cdl_path)
-        check_call(command, shell=True)
+        # Create a temporary netcdf file from the CDL string.
+        ncgen_from_cdl(ref_cdl, cdl_path, nc_path)
         # Load with iris.fileformats.netcdf.load_cubes, and check expected content.
         cubes = list(nc_load_cubes(nc_path))
         self.assertEqual(len(cubes), 1)

lib/iris/tests/unit/experimental/ugrid/load/test_load_meshes.py

@@ -13,7 +13,6 @@
 
 from pathlib import Path
 from shutil import rmtree
-from subprocess import check_call
 import tempfile
 from uuid import uuid4
 
@@ -22,6 +21,7 @@
     load_meshes,
     logger,
 )
+from iris.tests.stock.netcdf import ncgen_from_cdl
 
 
 def setUpModule():
@@ -35,15 +35,11 @@ def tearDownModule():
 
 
 def cdl_to_nc(cdl):
-    cdl_path = TMP_DIR / "tst.cdl"
-    nc_path = TMP_DIR / f"{uuid4()}.nc"
-    # Write CDL string into a temporary CDL file.
-    with open(cdl_path, "w") as f_out:
-        f_out.write(cdl)
+    cdl_path = str(TMP_DIR / "tst.cdl")
+    nc_path = str(TMP_DIR / f"{uuid4()}.nc")
     # Use ncgen to convert this into an actual (temporary) netCDF file.
-    command = "ncgen -o {} {}".format(nc_path, cdl_path)
-    check_call(command, shell=True)
-    return str(nc_path)
+    ncgen_from_cdl(cdl_str=cdl, cdl_path=cdl_path, nc_path=nc_path)
+    return nc_path
 
 
 class TestsBasic(tests.IrisTest):