Repository: pstelzig/MoPyRegtest (language: python)

Issue:
Automated test definition generation failing for non-buildable models
# Problem description

The command line interface fails to generate test definition files if the Modelica model can't be translated/simulated.

**To Reproduce**

The automated test definition breaks for `MyLibrary.M`:

```modelica
package MyLibrary "Package"
  model M "Model"
    Real x(fixed=true, start=0);
    real y;  // Syntax error and number of variables not matching number of equations
  equation
    der(x) = -0.5*x;
  end M;
end MyLibrary;
```

```bash
> mopyregtest generate .CI/mopyregtest/ "TestM" . MyLibrary.M
Simulating model MyLibrary.M using the simulation tools: omc
Using simulation tool omc
/bin/sh: 1: ./MyLibrary.M: not found
Traceback (most recent call last):
  File "/home/USERNAME/.local/bin/mopyregtest", line 8, in <module>
    sys.exit(main())
  File "/home/USERNAME/.local/lib/python3.10/site-packages/mopyregtest/cli.py", line 136, in main
    parse_args(sys.argv[1:])
  File "/home/USERNAME/.local/lib/python3.10/site-packages/mopyregtest/cli.py", line 129, in parse_args
    args.func(args)
  File "/home/USERNAME/.local/lib/python3.10/site-packages/mopyregtest/cli.py", line 48, in generate
    gen.generate_tests(test_folder, test_name, result_folder, references)
  File "/home/USERNAME/.local/lib/python3.10/site-packages/mopyregtest/generator.py", line 237, in generate_tests
    self._generate_reference(reference_folder=test_folder / "references",
  File "/home/USERNAME/.local/lib/python3.10/site-packages/mopyregtest/generator.py", line 175, in _generate_reference
    shutil.copyfile(pathlib.Path(tmp_res_folder) / res_file, str(pathlib.Path(reference_folder) / res_file))
  File "/usr/lib/python3.10/shutil.py", line 254, in copyfile
    with open(src, 'rb') as fsrc:
FileNotFoundError: [Errno 2] No such file or directory: '/tmp/MyLibrary.Mae09u5nf/MyLibrary.M_res.csv'
```

**System information (please complete the following information):**

 - OS: Ubuntu 22.04.3 LTS
 - OpenModelica: v1.23.0-dev-203-g5904d1ea84-cmake
 - Python: v3.10.12
 - MoPyRegtest: v0.5.0

# Desired behavior

Return a clear error message telling the user that:

  - the test definitions couldn't be generated for model `MyLibrary.M`.
  - print the error message from OpenModelica, giving a hint what the issue is and how to fix it.

The script should exit gracefully instead of failing to open some file and ideally clean up all half-generated files like `.CI/mopyregtest/references/MyLibrary.M_res.csv` and `.CI/mopyregtest/test_testm.py`.

So something like:

```bash
> mopyregtest generate .CI/mopyregtest/ "TestM" . MyLibrary.M
Failed to generate test definition for PNlib.TA:
Failed to build model:

[/path/to/MyLibrary/package.mo:4:5-4:11:writable] Error: Class real not found in scope M.

Aborting test definition generation.
Removing temporary files.
# exit code 1
```
Automated test definition generation hiding simulation errors
# Problem description

The command line interface fails to generate test definition files if the Modelica model can't be translated/simulated.

**To Reproduce**

The automated test definition generation hides simulation errors for `MyLibrary.M`:

```modelica
package MyLibrary "Package"
  model M "Model"
    Real x(fixed=true, start=0);
  equation
    der(x) = -0.5*x + sqrt(0.5 - time);  // Breaks simulation at time=0.5
  end M;
end MyLibrary;
```

```bash
> mopyregtest generate .CI/mopyregtest/ "TestM" . MyLibrary.M
Simulating model MyLibrary.M using the simulation tools: omc
Using simulation tool omc
```

The resulting reference file `.CI/mopyregtest/references/MyLibrary.M_res.csv` is missing half of the simulation, but `mopyregtest` exited without any errors.

**System information (please complete the following information):**

 - OS: Ubuntu 22.04.3 LTS
 - OpenModelica: v1.23.0-dev-203-g5904d1ea84-cmake
 - Python: v3.10.12
 - MoPyRegtest: v0.5.0

# Desired behavior

Return an error when the simulation for reference file generation failed.
Print or log the error message returned by OpenModelica.

So something like:

```
> mopyregtest generate .CI/mopyregtest/ "TestM" . MyLibrary.M
Simulating model MyLibrary.M using the simulation tools: omc
Using simulation tool omc

Failed to simulate model MyLibrary.M

LOG_STDOUT        | warning | A Modelica assert prevents the integrator to continue. For more information use -lv LOG_SOLVER
LOG_STDOUT        | warning | can't continue. time = 0.500000
LOG_STDOUT        | info    | model terminate | Integrator failed. | Simulation terminated at time 0.5

The full log can be viewed at some/path/MyLibrary.M.log
# exit code 1
```


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/job-example-for-other-repo.yml b/.github/workflows/job-example-for-other-repo.yml
index d8d34e0..7628953 100644
--- a/.github/workflows/job-example-for-other-repo.yml
+++ b/.github/workflows/job-example-for-other-repo.yml
@@ -3,7 +3,7 @@ on: [workflow_dispatch]
 jobs:
   examples-test:
     runs-on: ubuntu-latest
-    container: openmodelica/openmodelica:v1.22.1-minimal
+    container: openmodelica/openmodelica:v1.23.0-minimal
     steps:
       - name: Install dependencies
         run: |
@@ -13,7 +13,7 @@ jobs:
       - name: Install Modelica STL 4.0.0
         run: |
           echo "installPackage(Modelica, \"4.0.0+maint.om\", exactMatch=true);" > installModelicaStl.mos && omc installModelicaStl.mos
-      - name: Install MoPyRegtest with tag v0.5.0
+      - name: Install MoPyRegtest with tag v0.5.1-rc.1
         run: |
           git clone https://github.com/pstelzig/MoPyRegtest.git mopyregtest
           cd mopyregtest
diff --git a/.github/workflows/job-examples.yml b/.github/workflows/job-examples.yml
index 3f1c94f..ea3beab 100644
--- a/.github/workflows/job-examples.yml
+++ b/.github/workflows/job-examples.yml
@@ -3,7 +3,7 @@ on: [workflow_dispatch]
 jobs:
   examples-test:
     runs-on: ubuntu-latest
-    container: openmodelica/openmodelica:v1.22.1-minimal
+    container: openmodelica/openmodelica:v1.23.0-minimal
     steps:
       - uses: actions/checkout@v3
       - name: Install dependencies
diff --git a/.gitignore b/.gitignore
index 272215c..8d1f93e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -6,3 +6,5 @@ __pycache__
 build/
 MoPyRegtest.egg-info/
 /examples/test_user_defined_metrics/references/Sine_res_comparison.csv
+/tests/data/FlawedModels/results
+/tests/data/gentests
\ No newline at end of file
diff --git a/setup.py b/setup.py
index 0deb7aa..d998678 100644
--- a/setup.py
+++ b/setup.py
@@ -8,7 +8,7 @@
 
 setuptools.setup(
     name="MoPyRegtest",
-    version="0.5.0",
+    version="0.5.1-rc.1",
     author="Philipp Emanuel Stelzig",
     description="A Python enabled simple regression testing framework for Modelica models",
     long_description=long_description,


Test patch:
diff --git a/.github/workflows/job-unit-tests.yml b/.github/workflows/job-unit-tests.yml
index 87e8a13..dfd0616 100644
--- a/.github/workflows/job-unit-tests.yml
+++ b/.github/workflows/job-unit-tests.yml
@@ -3,7 +3,7 @@ on: [workflow_dispatch]
 jobs:
   unit-test:
     runs-on: ubuntu-latest
-    container: openmodelica/openmodelica:v1.22.1-minimal
+    container: openmodelica/openmodelica:v1.23.0-minimal
     strategy:
       matrix:
         python-version: ["3.8", "3.9", "3.10", "3.11"]
diff --git a/mopyregtest/generator.py b/mopyregtest/generator.py
index 4238464..ffbb9a3 100644
--- a/mopyregtest/generator.py
+++ b/mopyregtest/generator.py
@@ -10,6 +10,7 @@
 import os
 import pathlib
 import tempfile
+import sys
 
 from . import metrics
 from . import utils
@@ -168,11 +169,23 @@ def _generate_reference(self, reference_folder, model_in_package, tool="omc", do
                                  result_folder=tmp_res_folder, tool=tool,
                                  modelica_version=self.modelica_version,
                                  dependencies=self.dependencies)
-        regtest._import_and_simulate()
+
+        try:
+            regtest._import_and_simulate()
+        except Exception as e:
+            print(f"Failed to generate the test:\n\n{e}\n\n")
+            print("Aborting test generation.")
+            sys.exit(1)
 
         # Copy reference result file from temporary folder into reference target folder
         res_file = f"{model_in_package}_res.csv"
-        shutil.copyfile(pathlib.Path(tmp_res_folder) / res_file, str(pathlib.Path(reference_folder) / res_file))
+        ref_src = pathlib.Path(tmp_res_folder) / res_file
+        ref_dst = pathlib.Path(reference_folder) / res_file
+
+        if ref_dst.exists():
+            os.remove(ref_dst)
+
+        shutil.copyfile(ref_src, ref_dst)
 
         # Cleanup temporary directory with simulation data
         if do_cleanup:
diff --git a/mopyregtest/modelicaregressiontest.py b/mopyregtest/modelicaregressiontest.py
index 65f4f78..5b84bdc 100644
--- a/mopyregtest/modelicaregressiontest.py
+++ b/mopyregtest/modelicaregressiontest.py
@@ -7,6 +7,8 @@
 """
 
 import os
+import re
+import subprocess
 import platform
 import pathlib
 import shutil
@@ -49,11 +51,12 @@ def __init__(self, package_folder, model_in_package, result_folder, tool="omc",
             dependency is an entire package, it must be the path to the respective package's package.mo.
         """
 
+        self.initial_cwd = os.getcwd()
+
         self.template_folder_path = pathlib.Path(__file__).parent.absolute() / "templates"
-        self.package_folder_path = pathlib.Path(package_folder).absolute()
+        self.package_folder_path = self._make_path_absolut(package_folder)
         self.model_in_package = model_in_package
-        self.result_folder_path = pathlib.Path(result_folder).absolute()
-        self.initial_cwd = os.getcwd()
+        self.result_folder_path = self._make_path_absolut(result_folder)
         self.modelica_version = modelica_version
         self.dependencies = dependencies
 
@@ -64,6 +67,18 @@ def __init__(self, package_folder, model_in_package, result_folder, tool="omc",
 
         self.result_folder_created = False
 
+    def _make_path_absolut(self, path):
+        path = os.path.expanduser(path)
+
+        if not pathlib.PurePath(path).is_absolute():
+            path_abs = pathlib.Path(self.initial_cwd) / path
+        else:
+            path_abs = pathlib.Path(path)
+
+        path_abs = path_abs.absolute()
+
+        return path_abs
+
     @staticmethod
     def _unify_timestamps(results: List[pd.DataFrame], fill_in_method="ffill"):
         """
@@ -477,20 +492,20 @@ def _run_model(self):
                 utils.replace_in_file(self.result_folder_path / model_import_mos, repl_dict)
 
                 # Run the import script and write the output of the OpenModelica Compiler (omc) to omc_output
-                os.system(tool_executable + " {} > {}".format(model_import_mos, tool_output))
-
-                # Read simulation options from the simulation_options_file
-                model_import_output_file = open(tool_output, 'r')
-                omc_messages = model_import_output_file.readlines()
-                model_import_output_file.close()
+                proc_return = subprocess.run([tool_executable, model_import_mos], check=True, capture_output=True)
+                omc_messages = proc_return.stdout.decode("utf-8").strip("\'").strip("\n")
 
-                (start_time, stop_time, tolerance, num_intervals, interval) = omc_messages[-1].lstrip('(').rstrip(')').split(',')
+                (start_time, stop_time, tolerance, num_intervals, interval) = omc_messages.split("\n")[-1].lstrip('(').rstrip(')').split(',')
 
                 # Modify the simulation template
                 if platform.system() == 'Windows':
-                    repl_dict["SIMULATION_BINARY"] = "{}.exe".format(self.model_in_package)
+                    sim_binary = self.model_in_package + ".exe"
+                    repl_dict["SIMULATION_BINARY"] = sim_binary
                 elif platform.system() == 'Linux':
-                    repl_dict["SIMULATION_BINARY"] = "./{}".format(self.model_in_package)
+                    sim_binary = self.model_in_package
+                    repl_dict["SIMULATION_BINARY"] = "./" + sim_binary
+                else:
+                    raise ValueError(f"Platform {platform.system()} not supported")
                 repl_dict["START_TIME"] = start_time
                 repl_dict["STOP_TIME"] = stop_time
                 repl_dict["TOLERANCE"] = tolerance
@@ -498,9 +513,29 @@ def _run_model(self):
 
                 utils.replace_in_file(self.result_folder_path / model_simulate_mos, repl_dict)
 
-                # Run the simulation script and append the output of the OpenModelica Compiler (omc) to omc_output
-                os.system(tool_executable + " {} >> {}".format(model_simulate_mos, tool_output))
+                # Delete old simulation binary and old simulation result
+                sim_result_path = self.result_folder_path / (self.model_in_package + "_res.csv")
+                if sim_result_path.exists():
+                    os.remove(sim_result_path)
+
+                sim_binary_path = self.result_folder_path / sim_binary
+                if sim_binary_path.exists():
+                    os.remove(sim_binary_path)
 
+                # Run the simulation script and append the output of the OpenModelica Compiler (omc) to omc_output
+                proc_return = subprocess.run([tool_executable, model_simulate_mos], check=True, capture_output=True)
+                omc_messages = proc_return.stdout.decode("utf-8").strip("\'").strip("\n")
+
+                # Check output: Both simulation binary and simulation result must exist now
+                if not sim_binary_path.exists():
+                    raise AssertionError(
+                        f"The expected simulation binary at {sim_result_path} does not exist. "
+                        + f"Please check the output from the simulation tool:\n\n{omc_messages}")
+
+                if not sim_result_path.exists():
+                    raise AssertionError(
+                        f"The expected simulation result at {sim_result_path} does not exist. "
+                        + f"Please check the output from the simulation tool:\n\n{omc_messages}")
         return
 
     @staticmethod
diff --git a/tests/data/FlawedModels/package.mo b/tests/data/FlawedModels/package.mo
new file mode 100644
index 0000000..ae5fc54
--- /dev/null
+++ b/tests/data/FlawedModels/package.mo
@@ -0,0 +1,19 @@
+package FlawedModels "Modelica models that do not translate or do not run"
+    extends Modelica.Icons.ExamplesPackage;
+    model DoesNotBuild
+        extends Modelica.Icons.Example;
+        Real x(start = 0);
+        Real y(start = 1);
+    equation
+        der(x) = x*y;
+    end DoesNotBuild;
+
+    model DoesNotFinish
+        extends Modelica.Icons.Example;
+        Real x(start = 1);
+    equation
+        der(x) = sqrt(0.5-time);
+    annotation(
+      experiment(StartTime = 0, StopTime = 2, Tolerance = 1e-06, Interval = 0.002));
+end DoesNotFinish;
+end FlawedModels;
\ No newline at end of file
diff --git a/tests/test_cli.py b/tests/test_cli.py
index c907497..02c9d84 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -53,6 +53,35 @@ def test_generate3(self):
 
         return
 
+    def test_generate4(self):
+        """
+        Testing for generation to fail if the simulation model for which a test is
+        being generated fails to build.
+        """
+        cmd_args = ["generate",
+                    str(this_folder / "data/gentests"),
+                    "FlawedModels_DoesNotBuild",
+                    str(this_folder / "data/FlawedModels"),
+                    "FlawedModels.DoesNotBuild"]
+
+        self.assertRaises(AssertionError, cli.parse_args, cmd_args)
+
+        return
+
+    def test_generate5(self):
+        """
+
+        """
+        cmd_args = ["generate",
+                    str(this_folder / "data/gentests"),
+                    "FlawedModels_DoesNotFinish",
+                    str(this_folder / "data/FlawedModels"),
+                    "FlawedModels.DoesNotFinish"]
+
+        cli.parse_args(cmd_args)
+
+        return
+
     def test_compare1(self):
         """
         Testing for default arguments in CLI: mopyregtest compare
@@ -129,3 +158,7 @@ def test_compare5(self):
             (this_folder / "../examples/test_user_defined_metrics/references/Sine_res_comparison.csv").exists())
 
         return
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/test_compare_results.py b/tests/test_compare_results.py
index 455695b..31ee7b1 100644
--- a/tests/test_compare_results.py
+++ b/tests/test_compare_results.py
@@ -100,3 +100,7 @@ def test_write_comparison_timeseries2(self):
         self.assertTrue(compar_file_path.exists())
 
         return
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/test_metrics.py b/tests/test_metrics.py
index 63e7345..7a37063 100644
--- a/tests/test_metrics.py
+++ b/tests/test_metrics.py
@@ -150,4 +150,8 @@ def test_norm_infty_dist(self):
 
         self.assertRaises(ValueError, mopyregtest.metrics.norm_infty_dist, f1=f1, f2=f3)
 
-        self.assertRaises(ValueError, mopyregtest.metrics.norm_infty_dist, f1=f1, f2=f4)
\ No newline at end of file
+        self.assertRaises(ValueError, mopyregtest.metrics.norm_infty_dist, f1=f1, f2=f4)
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/test_modelerror.py b/tests/test_modelerror.py
new file mode 100644
index 0000000..64e9122
--- /dev/null
+++ b/tests/test_modelerror.py
@@ -0,0 +1,46 @@
+import os
+import unittest
+import pathlib
+import mopyregtest
+
+this_folder = pathlib.Path(__file__).absolute().parent
+initial_cwd = os.getcwd()
+
+class TestModelErrors(unittest.TestCase):
+    def test_build_errors(self):
+        # Exception in RegressionTest._import_and_simulate raised before RegressionTest switches back to initial_cwd
+        os.chdir(initial_cwd)
+
+        tester = mopyregtest.RegressionTest(package_folder=this_folder / "data/FlawedModels",
+                                          model_in_package="FlawedModels.DoesNotBuild",
+                                          result_folder=this_folder / "data/FlawedModels/results")
+
+        self.assertRaises(AssertionError, tester._import_and_simulate)
+
+        tester.cleanup(ask_confirmation=False)
+
+        return
+
+    def test_simulate_errors1(self):
+        """
+        This test runs a simulation model that will build and run, but the simulation will not reach the
+        targeted simulation end time. An error will _not_ be thrown by _import_and_simulate. This is because
+        for regression we do not care about simulation end times, but about reproducibility.
+        """
+
+        # Exception in RegressionTest._import_and_simulate raised before RegressionTest switches back to initial_cwd
+        os.chdir(initial_cwd)
+
+        tester = mopyregtest.RegressionTest(package_folder=this_folder / "data/FlawedModels",
+                                          model_in_package="FlawedModels.DoesNotFinish",
+                                          result_folder=this_folder / "data/FlawedModels/results")
+
+        tester._import_and_simulate()
+
+        tester.cleanup(ask_confirmation=False)
+
+        return
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/test_unify_timestamps.py b/tests/test_unify_timestamps.py
index 1fe50cb..cfd1ae7 100644
--- a/tests/test_unify_timestamps.py
+++ b/tests/test_unify_timestamps.py
@@ -99,4 +99,8 @@ def test_unify_timestamps_multiplicity(self):
         results_ext = mopyregtest.RegressionTest._unify_timestamps([res1, res2])
 
         self.assertIsNone(pd.testing.assert_frame_equal(res1_ext_expect, results_ext[0]))
-        self.assertIsNone(pd.testing.assert_frame_equal(res2_ext_expect, results_ext[1]))
\ No newline at end of file
+        self.assertIsNone(pd.testing.assert_frame_equal(res2_ext_expect, results_ext[1]))
+
+
+if __name__ == '__main__':
+    unittest.main()
\ No newline at end of file