Add testing for deploy and unit testing #17

mahf708 · 2025-04-25T18:47:05Z

Add additional tests targeting deploy testing and unit testing focusing on usability and robustness.

Unit Tests
- Add cacts/tests/test_build_type.py to test BuildType class, covering initialization, parameter updates, expand_variables, evaluate_commands, and str_to_bool methods.
- Add cacts/tests/test_cacts.py to test Driver class, covering initialization, run, generate_cmake_config, generate_ctest_cmd methods, and parse_command_line function.
- Add cacts/tests/test_machine.py to test Machine class, covering initialization, parameter updates, and uses_gpu method.
- Add cacts/tests/test_project.py to test Project class, covering initialization, parameter updates, and __post_init__ method.
- Add cacts/tests/test_utils.py to test utility functions in utils.py, covering expect, run_cmd, run_cmd_no_fail, expand_variables, evaluate_commands, str_to_bool, and is_git_repo functions.
Shell-based Tests
- Add cacts/tests/test_deploy.sh to cover deployment scenarios, including installing dependencies, running the application, and verifying the application's output and behavior.
CI Pipeline
- Update .github/workflows/test.yaml to include the new test files in the CI pipeline.
- Add a new step to run the shell-based test script test_deploy.sh.

For more details, open the Copilot Workspace session.

Add additional tests targeting deploy testing and unit testing focusing on usability and robustness. * **Unit Tests** - Add `cacts/tests/test_build_type.py` to test `BuildType` class, covering initialization, parameter updates, `expand_variables`, `evaluate_commands`, and `str_to_bool` methods. - Add `cacts/tests/test_cacts.py` to test `Driver` class, covering initialization, `run`, `generate_cmake_config`, `generate_ctest_cmd` methods, and `parse_command_line` function. - Add `cacts/tests/test_machine.py` to test `Machine` class, covering initialization, parameter updates, and `uses_gpu` method. - Add `cacts/tests/test_project.py` to test `Project` class, covering initialization, parameter updates, and `__post_init__` method. - Add `cacts/tests/test_utils.py` to test utility functions in `utils.py`, covering `expect`, `run_cmd`, `run_cmd_no_fail`, `expand_variables`, `evaluate_commands`, `str_to_bool`, and `is_git_repo` functions. * **Shell-based Tests** - Add `cacts/tests/test_deploy.sh` to cover deployment scenarios, including installing dependencies, running the application, and verifying the application's output and behavior. * **CI Pipeline** - Update `.github/workflows/test.yaml` to include the new test files in the CI pipeline. - Add a new step to run the shell-based test script `test_deploy.sh`. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/E3SM-Project/cacts?shareId=XXXX-XXXX-XXXX-XXXX).

.github/workflows/test.yaml

Copilot

Pull Request Overview

This PR adds a comprehensive suite of tests for deploy functionality and the core classes of the project to improve robustness and usability.

Added unit tests covering BuildType, Project, Machine, Driver, and various utility functions.
Introduced shell-based deployment tests.
Updated the CI pipeline to include the new tests.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
cacts/tests/test_utils.py	Adds tests for utility functions including shell command execution and variable expansion.
cacts/tests/test_project.py	Introduces tests for the Project class covering initialization and post-initialization.
cacts/tests/test_machine.py	Implements tests for Machine functionality including GPU usage verification.
cacts/tests/test_cacts.py	Provides tests for the Driver class and command-line parsing functions.
cacts/tests/test_build_type.py	Tests BuildType functionality including initialization, variable expansion, and command evaluation.
.github/workflows/test.yaml	Updates the CI pipeline to install dependencies and run both Python and shell-based tests.

Files not reviewed (1)

cacts/tests/test_deploy.sh: Language not supported

Comments suppressed due to low confidence (2)

cacts/tests/test_utils.py:9

The test assumes that run_cmd trims newline characters from the echo output; please verify that this behavior is consistent across different shell environments.

stat, output, errput = run_cmd("echo Hello, World!")

cacts/tests/test_build_type.py:41

Verify that the merging logic for cmake_args between the default and test_build specifications is intentional and handles potential key conflicts appropriately.

assert build_type.cmake_args == {'arg1': 'value1', 'arg2': 'value2'}

* **Run tests** - Update the `Run tests` step to use `uv` to run `pytest` * **Run shell-based deployment tests** - Update the `Run shell-based deployment tests` step to use `uv` to run `test_deploy.sh` * **Remove Install dependencies step** - Remove the step to install `uv` and set the python version

bartgol · 2025-04-25T19:32:08Z

cacts/utils.py

-    else:
-        return cpu_count
+        if platform.system() == "Darwin":  # macOS
+            cpu_count = os.cpu_count()  # Fallback for macOS


This returns all the cpus on the node. We may not want this.

bartgol · 2025-04-25T19:40:31Z

cacts/__main__.py

@@ -0,0 +1,4 @@
+from . import main


doesn't the pyproject.toml file already define the cacts program?

bartgol · 2025-04-25T19:44:37Z

cacts/tests/test_build_type.py

+    project = types.SimpleNamespace(name="TestProject")
+    machine = types.SimpleNamespace(name="TestMachine", env_setup=["echo 'Setting up environment'"])
+    builds_specs = {
+        'default': {


We should add some ${} and $() syntax in the specs, to make sure that evaluate_commands and expand_variables work correctly.

Also, given that ${} allow to execute py code, we should prob call it evaluate_py_expressions, and evaluate_commands should be evaluate_sh_expressions...or something. But that can wait.

I mean, we do have unit tests for expand/evaluate, but I wonder if that's enough, or if we should verify they are used correctly in the BT init...

bartgol · 2025-04-25T19:48:51Z

cacts/tests/test_machine.py

+def machine():
+    project = MockProject()
+    machines_specs = {
+        'default': {


maybe we can add some ${} and $() here too, and check that Machine init does the expansion correctly..

bartgol · 2025-04-25T19:51:26Z

cacts/tests/test_utils.py

+def test_run_cmd_no_fail():
+    output = run_cmd_no_fail("echo Hello, World!")
+    assert output == "Hello, World!"
+


I would also test that

Suggested change

with pytest.raises(RuntimeError):

run_cmd_no_fail("false")

to check that this throws.

bartgol

This is great! I have some comments. Not sure how much we want to refine now. Maybe it's better to add something now, and expand later..

mahf708 commented Apr 25, 2025

View reviewed changes

.github/workflows/test.yaml Outdated Show resolved Hide resolved

install the project instead of reqs

69e74ce

mahf708 requested a review from Copilot April 25, 2025 18:49

Copilot AI reviewed Apr 25, 2025

View reviewed changes

mahf708 added 2 commits April 25, 2025 14:54

remove some test

f41f7c5

mahf708 marked this pull request as draft April 25, 2025 19:29

bartgol reviewed Apr 25, 2025

View reviewed changes

cacts/__main__.py

@@ -0,0 +1,4 @@

from . import main

Copy link

Collaborator

bartgol Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't the pyproject.toml file already define the cacts program?

bartgol reviewed Apr 25, 2025

View reviewed changes

This was referenced Jul 31, 2025

Nano bug fix when running within SLURM env #26

Closed

add unit testing #29

Closed

Copilot AI mentioned this pull request Jul 31, 2025

Add comprehensive unit testing framework with code quality improvements #30

Merged

mahf708 closed this Jul 31, 2025

mahf708 deleted the mahf708/mahf708/add-testing branch July 31, 2025 18:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add testing for deploy and unit testing #17

Add testing for deploy and unit testing #17

Uh oh!

mahf708 commented Apr 25, 2025 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

bartgol Apr 25, 2025

Uh oh!

bartgol Apr 25, 2025

Uh oh!

bartgol Apr 25, 2025

Uh oh!

bartgol Apr 25, 2025 •

edited

Loading

Uh oh!

bartgol Apr 25, 2025

Uh oh!

bartgol Apr 25, 2025

Uh oh!

bartgol Apr 25, 2025

Uh oh!

bartgol left a comment

Uh oh!

Uh oh!

Add testing for deploy and unit testing #17

Add testing for deploy and unit testing #17

Uh oh!

Conversation

mahf708 commented Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

bartgol Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

bartgol Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

bartgol Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

bartgol Apr 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bartgol Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

bartgol Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

bartgol Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

bartgol left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mahf708 commented Apr 25, 2025 •

edited

Loading

bartgol Apr 25, 2025 •

edited

Loading