test: improve pre push hook error handling for new branches #51
Conversation
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds a comprehensive test script for pre-push hook error handling, specifically focusing on scenarios with new branches and various upstream configurations. The test script validates the hook's behavior in three key scenarios: branches without upstream, branches with upstream and package.json changes, and branches with upstream but non-monitored file changes.
Key changes:
- Added a comprehensive test script with real remote push testing
- Implemented proper cleanup mechanisms for test branches and files
- Added validation for hook behavior across different branch and file change scenarios
Comments suppressed due to low confidence (1)
test/test-pre-push-hook-focused.sh:230
- The error message is misleading - it says 'Aborting tests that require remote push' but the script exits completely on line 231, aborting all tests including the first one that doesn't require remote push.
log_error "Remote access check failed. Aborting tests that require remote push."
test/test-pre-push-hook-focused.sh
Outdated
| # Backup package.json | ||
| cp package.json package.json.backup | ||
|
|
||
| # Make initial change and commit |
There was a problem hiding this comment.
The sed command on line 105 is complex and could fail silently. Consider using a more robust approach with proper error handling or a simpler text modification method.
Copilot uses AI. Check for mistakes.
test/test-pre-push-hook-focused.sh
Outdated
| log_success "Successfully established upstream for test branch" | ||
|
|
||
| # Make another change to package.json to test hook with existing upstream | ||
| sed 's/"Test modification for pre-push hook testing"/"Another test modification for hook testing"/' package.json > package.json.tmp && mv package.json.tmp package.json |
There was a problem hiding this comment.
This sed command assumes the exact text from the previous modification exists. If the first sed command format changes, this will fail silently. Consider using a more robust text replacement method or adding validation.
Copilot uses AI. Check for mistakes.
🚀 Performance Benchmark Results
✅ All benchmarks passed! |
🚀 Performance Benchmark Results
✅ All benchmarks passed! |
This reverts commit 48fa4a3.
Co-authored-by: Copilot <[email protected]>
🚀 Performance Benchmark Results
✅ All benchmarks passed! |
🚀 Performance Benchmark Results
✅ All benchmarks passed! |
mfranzke
changed the title
Description
We'd like to test our
pre-pushGit hook.Type of Change
Testing
Checklist
Browser Testing
If applicable, please test in the following browsers:
Performance Impact
If this change affects performance:
Breaking Changes
If this introduces breaking changes, please describe:
Additional Notes
Any additional information that reviewers should know.