feat: replace bash integration test script with Python version #77
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the bash-based integration test script (integration_test.sh) with a Python implementation (integration_test.py), modernizing the build infrastructure while maintaining the same functionality for building SCORE modules and tracking warning counts.
Changes:
- Removed the 356-line bash script and replaced it with a 393-line Python script
- Updated the GitHub Actions workflow to invoke the Python script instead of the bash script
- Maintained all existing functionality including module building, warning/deprecation counting, and build summary generation
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| scripts/integration_test.sh | Complete removal of the bash-based integration test script |
| scripts/integration_test.py | New Python implementation providing the same integration test functionality with improved maintainability |
| .github/workflows/reusable_integration-build.yml | Updated workflow to call the Python script instead of the bash script |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Build command | ||
| cmd = ['bazel', 'build', '--verbose_failures', f'--config={config}'] + targets.split() | ||
| #cmd = ['ls'] # for testing purposes only, remove on PR |
There was a problem hiding this comment.
This line appears to be a commented-out testing artifact that should be removed before merging. It serves no purpose in production code and could cause confusion.
| #cmd = ['ls'] # for testing purposes only, remove on PR |
| old_identifier, old_link = get_identifier_and_link(old_module) | ||
|
|
||
| # Check if hash changed | ||
| hash_changed = old_module is None or old_module.hash != new_module.hash |
There was a problem hiding this comment.
When old_module is None, accessing old_module.hash in the second part of the OR expression will fail if Python evaluates both sides. While the short-circuit evaluation prevents this in practice, the logic is clearer and safer if restructured to: hash_changed = old_module is None or (old_module.hash is not None and old_module.hash != new_module.hash) or use a conditional that explicitly handles the None case first.
| hash_changed = old_module is None or old_module.hash != new_module.hash | |
| hash_changed = old_module is None or ( | |
| old_module.hash is not None and old_module.hash != new_module.hash | |
| ) |
| # Add tools directory to path to import Module | ||
| script_dir = Path(__file__).parent | ||
| repo_root = script_dir.parent | ||
| sys.path.insert(0, str(repo_root / "tools")) |
There was a problem hiding this comment.
Maybe it's time to merge scripts and tools? Or should we just Sontag in follow up PRs?
|
|
||
|
|
||
| # Build target groups - maps module names to their Bazel targets | ||
| BUILD_TARGET_GROUPS = { |
|
The created documentation from the pull request is available at: docu-html |
2 participants
No description provided.