Unit-tests per system/workload

[podkidyshev]

Summary

Tests structure now reflects the main sources folder structure with granular distribution:

• by system - tests/systems/<system name>
• by workload - tests/workloads/<workload name>

Test Plan

Unit tests are sufficient

Additional Notes

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• ✅ Review completed - (🔄 Check again to review again)

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ipod/unit-tests-order

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@tests/conftest.py`:
- Around line 188-224: The fixture nccl_tr's stdout_content is missing the
required marker that NCCLTestDefinition.was_run_successful() checks for; insert
the line "# Out of bounds values : 0 OK" into the multi-line stdout_content
(near where "# Avg bus bandwidth" is written) so was_run_successful() will
return True, and consider renaming one of the duplicate nccl_tr fixtures to
avoid confusion between the one in tests/conftest.py and the one in
tests/test_single_sbatch_runner.py.

[greptile-apps]

Greptile Overview

Greptile Summary

This PR successfully reorganizes the test suite to mirror the main source structure, creating a clearer, more maintainable test organization. Tests are now organized hierarchically by system type (tests/systems/<system>) and workload type (tests/workloads/<workload>), eliminating the previous flat structure with scattered directories like slurm_command_gen_strategy, json_gen_strategy, and job_status_retrieval_strategy.

Key Changes:

• Renamed/moved 82 test files with 97-100% similarity (all content preserved)
• Created 27 new __init__.py files to establish proper Python package structure
• Consolidated nccl_tr pytest fixture from tests/report_generation_strategy/conftest.py into root tests/conftest.py for better accessibility
• Removed old test directories (job_status_retrieval_strategy, json_gen_strategy, runai, slurm_command_gen_strategy, standalone_command_gen_strategy)
• Updated copyright headers from 2025 to 2026 where applicable
• Preserved all root-level tests (test_acceptance.py, test_cli.py, etc.) that test general infrastructure

Structure:

tests/
├── systems/           # System-specific tests
│   ├── kubernetes/
│   ├── lsf/
│   ├── runai/
│   ├── slurm/
│   └── standalone/
└── workloads/         # Workload-specific tests
    ├── nccl_test/
    ├── vllm/
    ├── jax_toolbox/
    └── [18 more workloads]

The refactoring is purely structural with no logic changes, improving test discoverability and maintainability.

Confidence Score: 5/5

• This PR is safe to merge with no risk
• Pure structural refactoring with no logic changes - all files moved with 97-100% similarity, proper package structure established with __init__.py files, old directories cleaned up correctly, and copyrights updated appropriately
• No files require special attention

Important Files Changed

Filename	Overview
tests/init.py	New empty __init__.py added with copyright header to make tests a proper Python package
tests/conftest.py	Moved nccl_tr fixture from tests/report_generation_strategy/conftest.py to root conftest for broader accessibility, copyright updated to 2026
tests/report_generation_strategy/conftest.py	Removed file - nccl_tr fixture moved to root conftest.py
tests/systems/init.py	New empty __init__.py added with copyright header for systems test package
tests/systems/kubernetes/test_system.py	Renamed from tests/test_kubernetes_system.py, copyright updated to 2024-2026
tests/systems/slurm/test_system.py	Renamed from tests/test_slurm_system.py, copyright updated to 2024-2026
tests/workloads/init.py	New empty __init__.py added with copyright header for workloads test package
tests/workloads/nccl_test/test_command_gen_strategy_slurm.py	Renamed from tests/slurm_command_gen_strategy/test_nccl_slurm_command_gen_strategy.py, copyright updated to 2024-2026
tests/workloads/vllm/test_workload.py	Renamed from tests/test_vllm.py, no code changes

_{Last reviewed commit: b95ecf7}

[greptile-apps]

_{83 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/workloads/nccl_test/test_performance_report_gen_strategy.py (1)

45-53: ⚠️ Potential issue | 🟡 Minor

Remove explicit assertion messages to let pytest introspection work.
Use plain asserts; the messages add noise and are discouraged in this repo.

Suggested update

-    assert csv_report_path.is_file(), "CSV report was not generated."
+    assert csv_report_path.is_file()

-    assert not df.empty, "CSV report is empty."
+    assert not df.empty

-    assert df["Size (B)"].dtype == int, "Size (B) is not an integer."
-    assert df["Time (us) Out-of-place"].dtype == float, "Time (us) Out-of-place is not a float."
-    assert df["Time (us) In-place"].dtype == float, "Time (us) In-place is not a float."
+    assert df["Size (B)"].dtype == int
+    assert df["Time (us) Out-of-place"].dtype == float
+    assert df["Time (us) In-place"].dtype == float

-    assert df["GPU Type"].iloc[0] == "H100", "GPU Type was not extracted correctly."
-    assert df["Devices per Node"].iloc[0] == 8, "Devices per Node is incorrect."
-    assert df["Ranks"].iloc[0] == 16, "Ranks is incorrect."
+    assert df["GPU Type"].iloc[0] == "H100"
+    assert df["Devices per Node"].iloc[0] == 8
+    assert df["Ranks"].iloc[0] == 16

-            assert gpu_type == expected_type, f"Failed to parse GPU type for {gpu_line}"
+            assert gpu_type == expected_type

As per coding guidelines, "In pytest, rely on assertion introspection for failure details and avoid including explicit values in the assertion message (e.g., avoid f\"expected {x!r}, got {y!r}\"). Let pytest show actual vs expected. You may add concise context messages only when the assertion is not easily understandable from the expression alone."

Also applies to: 70-72, 97-97