Add comprehensive unit tests for core modules #169
Conversation
Added extensive unit tests for aws_login, file_io, saml2aws_helper, and selector modules to improve code coverage and reliability. Tests cover various edge cases, error handling, and expected behaviors for CLI commands, file operations, and helper functions.
|
⏳ Code review in progress. Analyzing for code quality issues and best practices. Detailed findings will be posted upon completion. Using Amazon Q Developer for GitHubAmazon Q Developer1 is an AI-powered assistant that integrates directly into your GitHub workflow, enhancing your development process with intelligent features for code development, review, and transformation. Slash Commands
FeaturesAgentic Chat Code Review CustomizationYou can create project-specific rules for Amazon Q Developer to follow:
Example rule: FeedbackTo provide feedback on Amazon Q Developer, create an issue in the Amazon Q Developer public repository. For more detailed information, visit the Amazon Q for GitHub documentation. Footnotes
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overall, this is a well-structured addition of comprehensive unit tests that significantly improves the project's test coverage. The tests are well-organized and cover important functionality including edge cases.
Key observations:
- Good coverage of core functionality in aws_login, file_io, saml2aws_helper, and selector modules
- Effective use of mocking for external dependencies
- Clear test organization using test classes
Suggestions for improvement:
- Add tests for invalid input handling and edge cases
- Include security-focused test cases for file operations
- Consider adding concurrency tests
- Add tests for international character support
Please address the inline comments to further strengthen the test suite. The changes look good to merge once the suggested improvements are implemented.
| role_arn, account_alias, "RoleName" | ||
| ) | ||
| assert result == "team-subteam-dev" | ||
|
|
There was a problem hiding this comment.
Consider adding test cases for invalid role ARN formats to ensure the function handles malformed input gracefully. This would help catch potential issues with unexpected ARN formats.
| def test_run_saml2aws_list_roles_parse_error(self, mock_popen, mock_getpass, mock_load_config): | ||
| mock_load_config.return_value = {"username": "testuser"} | ||
| mock_getpass.return_value = "testpass" | ||
|
|
There was a problem hiding this comment.
Consider adding test cases for error handling when subprocess.Popen raises exceptions (e.g., command not found, permission denied). This would improve the robustness of the test suite.
| f.write(" line1 \n line2 \n line3 \n") | ||
| temp_file = f.name | ||
|
|
||
| try: |
There was a problem hiding this comment.
🛑 Security Vulnerability: The test file operations should be performed in a secure temporary directory to prevent path traversal vulnerabilities1. Consider using tempfile.mkdtemp() with appropriate permissions.
Footnotes
-
CWE-22: Path Traversal - https://cwe.mitre.org/data/definitions/22.html ↩
|
|
||
|
|
||
| class TestPromptProfileSelection: | ||
| @patch('saml2awsmulti.selector.inquirer') |
There was a problem hiding this comment.
Consider adding test cases for handling special characters and Unicode in profile/role names. This would ensure the selector works correctly with international characters and special symbols.
| assert len(result) == 2 | ||
|
|
||
| @patch('saml2awsmulti.aws_login.exists') | ||
| @patch('saml2awsmulti.aws_login.read_csv') |
There was a problem hiding this comment.
Consider adding test cases for concurrent access to the credentials file. This would help ensure thread safety when multiple processes try to access AWS credentials simultaneously.
Refactored test cases in test_aws_login.py to assert log messages using pytest's caplog fixture instead of checking result.output. Also updated a test in test_saml2aws_helper.py to expect KeyError instead of ValueError. Added junit.xml to .gitignore.
Replaces setup.py, requirements files, and tox.ini with pyproject.toml and Makefile for dependency management, testing, and building. Updates GitHub Actions workflow and README to use Makefile targets. This modernizes the project structure and simplifies development and CI processes.
Added explicit Python 3.13 setup to the GitHub Actions workflow and removed Python 3.10 from the test matrix. Updated Makefile to check for 'uv' before running commands and made dependency installation targets depend on 'check-uv'. Added 'coverage.xml' to .gitignore.
Removed the 'run' target from the Makefile as awslogin is an interactive CLI tool. Updated the README to clarify build, install, and run steps, and improved documentation for available Makefile targets and testing commands.
Replaced uv with Poetry for dependency management and updated related documentation, Makefile targets, and CI workflows. Added poetry.lock, removed MANIFEST.in and constraints.txt, and updated .gitignore. Enhanced Dependabot and GitHub Actions to support Poetry. Updated CHANGELOG and README to reflect the new build and install process.
Added yamllint as a development dependency in pyproject.toml and updated the Makefile and GitHub Actions workflow to use Poetry for installing and running yamllint. Also renamed the yamllint config file for consistency and made minor improvements to the linting process.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #169 +/- ##
===========================================
+ Coverage 33.17% 98.92% +65.74%
===========================================
Files 4 8 +4
Lines 205 926 +721
===========================================
+ Hits 68 916 +848
+ Misses 137 10 -127 🚀 New features to boost your workflow:
|
Added extensive unit tests for aws_login, file_io, saml2aws_helper, and selector modules to improve code coverage and reliability. Tests cover various edge cases, error handling, and expected behaviors for CLI commands, file operations, and helper functions.