Merge pull request #272 from NOAA-GFDL/fre-pp-click-fix-n-test

`fre pp configure-yaml` entry-point for click fix, test

fre/app/regrid_xy/regrid_xy.py

@@ -164,7 +164,7 @@ def regrid_xy(input_dir = None, output_dir = None, begin = None, tmp_dir = None,
     """
 
     ## rose config load check
-    config_name = os.getcwd()
+    config_name = os.getcwd() #REMOVE ME TODO
     config_name += '/rose-app-run.conf'
     #config_name += '/rose-app.conf'
     print(f'config_name = {config_name}')
@@ -235,8 +235,6 @@ def regrid_xy(input_dir = None, output_dir = None, begin = None, tmp_dir = None,
 
 
     # grid_spec file management
-    #starting_dir = os.getcwd()
-    #os.chdir(work_dir) # i hate it
     if '.tar' in grid_spec:
         untar_sp = \
             subprocess.run( ['tar', '-xvf', grid_spec, '-C', input_dir],
@@ -463,7 +461,6 @@ def regrid_xy(input_dir = None, output_dir = None, begin = None, tmp_dir = None,
 
         continue # end of comp loop, exit or next one.
 
-    #os.chdir(starting_dir) # not clear this is necessary.
     print('done running regrid_xy()')
     return 0
 

fre/make/tests/test_create_makefile.py

@@ -7,7 +7,7 @@
 from fre.make import createMakefile
 
 # SET-UP
-test_dir = Path("fre/make/tests")
+TEST_DIR = Path("fre/make/tests")
 NM_EXAMPLE = Path("null_example")
 YAMLFILE = "null_model.yaml"
 BM_PLATFORM = ["ncrc5.intel23"]
@@ -16,57 +16,60 @@
 EXPERIMENT = "null_model_full"
 
 # Create output location
-out = f"{test_dir}/makefile_out"
-if Path(out).exists():
+OUT = f"{TEST_DIR}/makefile_out"
+if Path(OUT).exists():
     # remove
-    shutil.rmtree(out)
+    shutil.rmtree(OUT)
     # create output directory
-    Path(out).mkdir(parents=True,exist_ok=True)
+    Path(OUT).mkdir(parents=True,exist_ok=True)
 else:
-    Path(out).mkdir(parents=True,exist_ok=True)
+    Path(OUT).mkdir(parents=True,exist_ok=True)
 
 # Set output directory as home for fre make output
-#os.environ["HOME"]=str(Path(out))
+#os.environ["HOME"]=str(Path(OUT))
 
 def test_modelyaml_exists():
     """
     Check the model yaml exists
     """
-    assert Path(f"{test_dir}/{NM_EXAMPLE}/{YAMLFILE}").exists()
+    assert Path(f"{TEST_DIR}/{NM_EXAMPLE}/{YAMLFILE}").exists()
 
 def test_compileyaml_exists():
     """
     Check the compile yaml exists
     """
-    assert Path(f"{test_dir}/{NM_EXAMPLE}/compile.yaml").exists()
+    assert Path(f"{TEST_DIR}/{NM_EXAMPLE}/compile.yaml").exists()
 
 def test_platformyaml_exists():
     """
     Check the platform yaml exists
     """
-    assert Path(f"{test_dir}/{NM_EXAMPLE}/platforms.yaml").exists()
+    assert Path(f"{TEST_DIR}/{NM_EXAMPLE}/platforms.yaml").exists()
 
 def test_bm_makefile_creation():
     """
     Check the makefile is created when a bare-metal platform is used
     """
     # Set output directory as home for fre make output
-    os.environ["HOME"]=str(Path(out))
+    def_home = str(os.environ["HOME"])
+    os.environ["HOME"]=OUT#str(Path(OUT))
 
     bm_plat = BM_PLATFORM[0]
     targ = TARGET[0]
-    yamlfile_path = f"{test_dir}/{NM_EXAMPLE}/{YAMLFILE}"
+    yamlfile_path = f"{TEST_DIR}/{NM_EXAMPLE}/{YAMLFILE}"
 
     createMakefile.makefile_create(yamlfile_path,BM_PLATFORM,TARGET)
 
-    assert Path(f"{out}/fremake_canopy/test/{EXPERIMENT}/{bm_plat}-{targ}/exec/Makefile").exists()
+    assert Path(f"{OUT}/fremake_canopy/test/{EXPERIMENT}/{bm_plat}-{targ}/exec/Makefile").exists()
+    os.environ["HOME"] = def_home
+    assert os.environ["HOME"] == def_home
 
 def test_container_makefile_creation():
     """
     Check the makefile is created when the container platform is used
     """
     container_plat = CONTAINER_PLATFORM[0]
-    yamlfile_path = f"{test_dir}/{NM_EXAMPLE}/{YAMLFILE}"
+    yamlfile_path = f"{TEST_DIR}/{NM_EXAMPLE}/{YAMLFILE}"
     createMakefile.makefile_create(yamlfile_path,CONTAINER_PLATFORM,TARGET)
 
     assert Path(f"tmp/{container_plat}/Makefile").exists()

fre/pp/checkoutScript.py

@@ -16,7 +16,7 @@
 
 #############################################
 
-def _checkoutTemplate(experiment, platform, target, branch='main'):
+def checkoutTemplate(experiment, platform, target, branch='main'):
     """
     Checkout the workflow template files from the repo
     """
@@ -25,7 +25,7 @@ def _checkoutTemplate(experiment, platform, target, branch='main'):
     os.makedirs(directory, exist_ok=True)
 
     # Change the current working directory
-    os.chdir(directory)
+    #os.chdir(directory)
 
     # Set the name of the directory
     name = f"{experiment}__{platform}__{target}"
@@ -34,7 +34,7 @@ def _checkoutTemplate(experiment, platform, target, branch='main'):
     click.echo("cloning experiment into directory " + directory + "/" + name)
     clonecmd = (
         f"git clone -b {branch} --single-branch --depth=1 --recursive "
-        f"https://github.com/NOAA-GFDL/fre-workflows.git {name}" )
+        f"https://github.com/NOAA-GFDL/fre-workflows.git {directory}/{name}" )
     preexist_error = f"fatal: destination path '{name}' exists and is not an empty directory."
     click.echo(clonecmd)
     cloneproc = subprocess.run(clonecmd, shell=True, check=False, stdout=PIPE, stderr=STDOUT)
@@ -43,8 +43,8 @@ def _checkoutTemplate(experiment, platform, target, branch='main'):
             argstring = f" -e {experiment} -p {platform} -t {target}"
             stop_report = (
                 "Error in checkoutTemplate: the workflow definition specified by -e/-p/-t already"
-                f" exists at the location ~/cylc-src/{name}!\n"
-                f"In the future, we will confirm that ~/cylc-src/{name} is usable and will check "
+                f" exists at the location {directory}/{name}!\n"
+                f"In the future, we will confirm that {directory}/{name} is usable and will check "
                 "whether it is up-to-date.\n"
                 "But for now, if you wish to proceed, you must delete the workflow definition.\n"
                 "To start over, try:\n"
@@ -62,12 +62,12 @@ def _checkoutTemplate(experiment, platform, target, branch='main'):
 #############################################
 
 @click.command()
-def checkoutTemplate(experiment, platform, target, branch="main"):
+def _checkoutTemplate(experiment, platform, target, branch="main"):
     '''
     Wrapper script for calling checkoutTemplate - allows the decorated version
     of the function to be separate from the undecorated version
     '''
-    return _checkoutTemplate(experiment, platform, target, branch)
+    return checkoutTemplate(experiment, platform, target, branch)
 
 
 if __name__ == '__main__':

fre/pp/frepp.py

@@ -1,10 +1,10 @@
 ''' fre pp '''
 
 import click
-from .checkoutScript import checkoutTemplate
-from .configure_script_yaml import yamlInfo
+from .checkoutScript import _checkoutTemplate
+from .configure_script_yaml import _yamlInfo
 from .configure_script_xml import convert
-from .validate import validate_subtool
+from .validate import _validate_subtool
 from .install import install_subtool
 from .run import pp_run_subtool
 from .status import status_subtool
@@ -64,7 +64,7 @@ def run(context, experiment, platform, target):
 def validate(context, experiment, platform, target):
     # pylint: disable=unused-argument
     """ - Validate PP configuration"""
-    context.forward(validate_subtool)
+    context.forward(_validate_subtool)
 
 # fre pp install
 @pp_cli.command()
@@ -100,7 +100,7 @@ def install(context, experiment, platform, target):
 def configure_yaml(context,yamlfile,experiment,platform,target):
     # pylint: disable=unused-argument
     """ - Execute fre pp configure """
-    context.forward(yamlInfo)
+    context.forward(_yamlInfo)
 
 @pp_cli.command()
 @click.option("-e", "--experiment", type=str,
@@ -122,7 +122,7 @@ def configure_yaml(context,yamlfile,experiment,platform,target):
 def checkout(context, experiment, platform, target, branch='main'):
     # pylint: disable=unused-argument
     """ - Execute fre pp checkout """
-    context.forward(checkoutTemplate)
+    context.forward(_checkoutTemplate)
 
 @pp_cli.command()
 @click.option('-x', '--xml',

fre/pp/validate.py

@@ -5,12 +5,12 @@
 import subprocess
 import click
 
-def _validate_subtool(experiment, platform, target):
+def validate_subtool(experiment, platform, target):
     """
     Validate the Cylc workflow definition located in
     ~/cylc-src/<experiment>__<platform>__<target>
     """
-
+    go_back_here = os.getcwd()
     directory = os.path.expanduser('~/cylc-src/' + experiment + '__' + platform + '__' + target)
 
     # Change the current working directory
@@ -23,8 +23,9 @@ def _validate_subtool(experiment, platform, target):
     # Validate the Cylc configuration
     cmd = "cylc validate ."
     subprocess.run(cmd, shell=True, check=True)
+    os.chdir(go_back_here)
 
 @click.command()
-def validate_subtool(experiment, platform, target):
+def _validate_subtool(experiment, platform, target):
     ''' entry point to validate for click '''
-    return _validate_subtool(experiment, platform, target)
+    return validate_subtool(experiment, platform, target)

fre/tests/test_fre_pp_cli.py

@@ -1,5 +1,8 @@
 ''' test "fre pp" calls '''
 
+import os
+from pathlib import Path
+
 from click.testing import CliRunner
 
 from fre import fre
@@ -39,6 +42,16 @@ def test_cli_fre_pp_checkout_opt_dne():
     result = runner.invoke(fre.fre, args=["pp", "checkout", "optionDNE"])
     assert result.exit_code == 2
 
+def test_cli_fre_pp_checkout_case():
+    ''' fre pp checkout -e FOO -p BAR -t BAZ'''
+    result = runner.invoke(fre.fre, args=["pp", "checkout", 
+                                          "-e", "FOO", 
+                                          "-p", "BAR", 
+                                          "-t", "BAZ"] )
+    directory = os.path.expanduser("~/cylc-src")+'/FOO__BAR__BAZ'
+    assert all( [ result.exit_code == 0,
+                  Path(directory).exists()] )                           
+
 #-- fre pp configure-xml
 def test_cli_fre_pp_configure_xml():
     ''' fre pp configure-xml '''
@@ -71,6 +84,18 @@ def test_cli_fre_pp_configure_yaml_opt_dne():
     result = runner.invoke(fre.fre, args=["pp", "configure-yaml", "optionDNE"])
     assert result.exit_code == 2
 
+def test_cli_fre_pp_configure_yaml_fail1():
+    ''' fre pp configure-yaml '''
+    result = runner.invoke(fre.fre, args = [ "pp", "configure-yaml",
+                                             "-e", "FOO",
+                                             "-p", "BAR",
+                                             "-t", "BAZ",
+                                             "-y", "BOO"              ] )
+    assert all( [ result.exit_code == 1,
+                  isinstance(result.exception, FileNotFoundError )
+               ] )
+
+
 #-- fre pp install
 def test_cli_fre_pp_install():
     ''' fre pp install '''

fre/yamltools/combine_yamls.py

@@ -143,10 +143,12 @@ def combine_model(self):
         f1.write(f'name: &name "{self.name}"\n')
         f1.write(f'platform: &platform "{self.platform}"\n')
         f1.write(f'target: &target "{self.target}"\n\n')
-        with open(self.yml,'r',encoding='UTF-8') as f2:
-            f1.write("### MODEL YAML SETTINGS ###\n")
-            shutil.copyfileobj(f2,f1)
-
+        try:
+            with open(self.yml,'r',encoding='UTF-8') as f2:
+                f1.write("### MODEL YAML SETTINGS ###\n")
+                shutil.copyfileobj(f2,f1)
+        except Exception as exc:
+            raise FileNotFoundError(f'{self.yml} not found') from exc
     print(f"   model yaml: {self.yml}")
 
   def combine_compile(self):
@@ -231,10 +233,12 @@ def combine_model(self):
         f1.write(f'name: &name "{self.name}"\n')
         f1.write(f'platform: &platform "{self.platform}"\n')
         f1.write(f'target: &target "{self.target}"\n\n')
-        with open(self.yml,'r',encoding='UTF-8') as f2:
-            f1.write("### MODEL YAML SETTINGS ###\n")
-            shutil.copyfileobj(f2,f1)
-
+        try:
+            with open(self.yml,'r',encoding='UTF-8') as f2:
+                f1.write("### MODEL YAML SETTINGS ###\n")
+                shutil.copyfileobj(f2,f1)
+        except Exception as exc:
+            raise FileNotFoundError(f'{self.yml} not found') from exc
     print(f"   model yaml: {self.yml}")
 
   def combine_experiment(self):