Add functional tests #69
Conversation
WalkthroughThis pull request introduces several changes to the must-gather-explorer project, focusing on test infrastructure and utility function modifications. A new constant Changes
Possibly related issues
Possibly related PRs
Suggested labels
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Report bugs in Issues The following are automatically added:
Available user actions:
Supported /retest check runs
Supported labels
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
must_gather_explorer/tests/test_functional.py (3)
1-2: Unused import note.
Currently, there's no direct usage ofsysat lines 1-2, except in theCapturingcontext manager. This is fine, but keep an eye on import usage to maintain cleanliness.
29-31: Suggestion: Add explicit assertions.
test_get_aliases_file_pathcallsget_aliases_file_pathbut does not assert any condition. Consider adding an assertion to verify that the returned path is valid or resolvable.
55-57: Alias resolution check.
test_get_resource_kind_by_aliasensures that recognized aliases map to valid kinds. Consider validating an unrecognized alias to confirm the function handles invalid inputs.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
must_gather_explorer/tests/constants.py(1 hunks)must_gather_explorer/tests/test_cli.py(2 hunks)must_gather_explorer/tests/test_functional.py(1 hunks)must_gather_explorer/utils.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- must_gather_explorer/tests/constants.py
🔇 Additional comments (9)
must_gather_explorer/tests/test_cli.py (2)
6-6: Good improvement: Centralizing the must-gather path.
This import of MUST_GATHER_PATH_FOR_TESTS promotes maintainability by removing the previously hardcoded value and referencing a constant instead, which makes updates easier if the test data path changes.
44-44: Flexible test setup confirmed.
By using MUST_GATHER_PATH_FOR_TESTS in the yaml_files fixture, you're ensuring consistent, centralized configuration for the path, preventing potential path mismatches across different tests.
must_gather_explorer/tests/test_functional.py (6)
3-12: Consistent usage of constants in tests.
Importing MUST_GATHER_PATH_FOR_TESTS here aligns this file with the same approach used in other test files. This ensures a consistent reference to test data across all tests in the suite.
17-27: Neat solution for capturing stdout.
The Capturing context manager is well-implemented and consistent with Pythonic patterns. This approach helps ensure logs and console outputs can be tested effectively.
33-35: Sufficient coverage for reading aliases.
This test ensures read_aliases_file returns a non-empty result. It might be worthwhile to add more validations in the future (e.g., check for specific keys).
37-41: Comprehensive coverage for file presence.
Ensuring that both YAML and log files are identified is crucial for verifying must-gather data is properly parsed. This test is solid.
43-52: Robust resource validation.
Your checks confirm essential metadata fields (name, namespace, yaml_file) are populated. This is a strong approach to guaranteeing the resource data’s integrity.
59-74: Functional coverage for resource retrieval output.
By capturing and verifying console output, you effectively ensure that get_resources displays the expected data. The test can easily be extended to parametrize and cover other resource kinds.
must_gather_explorer/utils.py (1)
209-209: Optional parameters: Excellent flexibility.
Changing name and namespace parameters to optional strings ("") is a good design choice, allowing calls to omit these arguments when not needed. Ensure you have tests covering these optional parameter use cases to confirm no regressions or unexpected resource filtering.
Would you like a script to confirm coverage of calls to get_cluster_resources_raw_data with and without passing name/namespace?
Summary by CodeRabbit
New Features
Bug Fixes
Tests