Skip to content

Feature/6 testing suite#11

Merged
MiguelR222 merged 7 commits into
mainfrom
Feature/6-testing-suite
Aug 27, 2025
Merged

Feature/6 testing suite#11
MiguelR222 merged 7 commits into
mainfrom
Feature/6-testing-suite

Conversation

@MiguelR222
Copy link
Copy Markdown
Contributor

Features:

  • Add testing suite
  • Add github actions testing workflow

Fix:

  • Convert cores to float on line 38 on cli/app.py

Closes: #6

Copy link
Copy Markdown
Contributor

@sidsun1 sidsun1 left a comment

Choose a reason for hiding this comment

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

LGTM overall, though I had some comments and recommended changes before merging to main!

Great work on the github test workflow. Everything here looks great. This wasn't explicitly mentioned before, but I'd appreciate if you could add a linting/format test with ruff. Reference #12 for further context.

As for the cli changes and testing I did have a couple of questions. At this moment, I don't think that the is_falsy_value function is too helpful as I believe Typer automatically checks for null or bad input purely based on type hints. These checks can be parameterized in the test modules (more on this later).

For the test file, I think it is well written overall. I do think that you can do can do a couple of things to make it a bit more modular.

  1. monkeypatch is a good approach for mocking/patching, but given you modify attributes and run things at a granular level, is there perhaps a better approach? If not, that's totally okay.
  2. I think asserting all the unpacked values into just one tuple makes it easier and more compact. (For example assert (total, used, free_mem) == ('764', '492', '144')). Similarly, since you are checking all of those lines in output in the TestCLI class, consider looping through a list of expected output strings.
  3. Pytest offers very nice reusability. I think it would be a good choice to run a fixture like @pytest.fixture on the CliRunner obj if we are to change up the codebase to have multiple test files. We can also use the decorator @pytest.mark.parameterize for all of those commands being run in test_monitor_default_outputs_sections. We would just need one monkeypatch.setattr and would allow us to write more tests within that same function.

Again, great work, these are just small nitpicks that will direct the overall structure of the project!

…@pytest.mark.parametize, make assertions for values a tuple instead of separate assertions
Copy link
Copy Markdown
Contributor

@sidsun1 sidsun1 left a comment

Choose a reason for hiding this comment

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

New changes on both commits LGTM!

@MiguelR222 MiguelR222 merged commit d6b3efa into main Aug 27, 2025
1 check passed
@MiguelR222 MiguelR222 deleted the Feature/6-testing-suite branch August 27, 2025 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: Add Testing Suite and CI/CD

2 participants