-
Notifications
You must be signed in to change notification settings - Fork 119
'gh ado2gh rewire-pipeline': enable test mode before actual rewiring #1417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… the migration process. 1. Enhanced Branch Policy Failure Logging ✅ Added detailed logging to CheckBranchPolicyRequirement method: Logs when repository name is not available Logs specific error messages for each type of failure: Network/HTTP errors Timeout errors JSON parsing errors Invalid argument errors Invalid operation errors Each log includes the pipeline ID, organization, team project, and repository for easy identification ✅ Enhanced IsPipelineRequiredByBranchPolicy method with verbose logging: Logs when repository ID cannot be found Logs when no branch policies exist (verbose) Logs when a pipeline IS required by branch policy (verbose) Logs when a pipeline is NOT required by branch policy (verbose) Logs specific error details for HTTP, JSON, and argument errors ✅ Added migration summary logging to ChangePipelineRepo method: Provides clear, informational messages about branch policy check results Explains the trigger configuration decision being made Helps users understand whether their branch policies were preserved.
- Introduced `--dry-run` option for single pipeline testing, allowing temporary rewiring to GitHub and build execution. - Implemented `test-pipelines` command for batch testing of multiple Azure DevOps pipelines with concurrency control. - Added `PipelineTestResult` and `PipelineTestSummary` models for detailed reporting of test results. - Enhanced `RewirePipelineCommand` to include new options for dry-run and monitoring timeout. - Created comprehensive documentation for pipeline testing feature, including usage examples and error handling. - Implemented logging and error handling for restoration processes during testing.
…equest validation - Implemented tests for IsPipelineRequiredByBranchPolicy method to verify behavior when pipeline is required, not in policy, policy is disabled, and no build policies exist. - Added tests for RewirePipelineToGitHub method to ensure triggers are preserved or enabled based on branch policy requirements. - Created AdoPipelineTriggerServiceFactory to facilitate the creation of AdoPipelineTriggerService instances with dependency injection. - Enhanced test coverage for CreateYamlControlledTriggers and HasPullRequestTrigger methods to validate trigger creation logic.
… tests for improved readability and error handling
… GitHub integration
…functionality - Merged AdoPipelineTriggerService from main for comprehensive pipeline trigger management - Preserved dry-run functionality with PipelineTestService - Updated RewirePipelineCommandHandler to support both regular rewiring and dry-run testing - Constructor now accepts AdoPipelineTriggerService for trigger preservation logic - Dry-run mode uses PipelineTestService for temporary testing workflow
…mproved pipeline rewiring logic and update related tests for better integration
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive test mode functionality to the gh ado2gh rewire-pipeline
command that allows validation of Azure DevOps pipelines before permanently migrating them to GitHub.
- Adds
--dry-run
flag to therewire-pipeline
command for single pipeline testing - Introduces new
test-pipelines
command for batch testing multiple pipelines concurrently - Implements pipeline test monitoring with automatic restoration to ensure pipelines are always restored to their original ADO repositories
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/ado2gh/Program.cs | Registers new AdoPipelineTriggerServiceFactory dependency |
src/ado2gh/Factories/AdoPipelineTriggerServiceFactory.cs | Factory for creating AdoPipelineTriggerService instances |
src/ado2gh/Commands/TestPipelines/* | New command implementation for batch pipeline testing |
src/ado2gh/Commands/RewirePipeline/* | Updates to support dry-run mode and use new trigger service |
src/Octoshift/Services/PipelineTestService.cs | Service for testing individual pipelines |
src/Octoshift/Services/AdoPipelineTriggerService.cs | Service for managing pipeline trigger configurations |
src/Octoshift/Services/AdoApi.cs | Adds new API methods for pipeline testing operations |
src/Octoshift/Models/* | New models for pipeline test results and ADO branch policies |
test files | Comprehensive test coverage for new functionality |
docs/pipeline-testing.md | Documentation for the new testing features |
Comments suppressed due to low confidence (3)
src/Octoshift/Services/AdoPipelineTriggerService.cs:431
- The method returns 'triggers' instead of 'result' which is the variable being built up in this method. This will return the original triggers array instead of the new YAML-controlled triggers.
}
src/Octoshift/Services/AdoPipelineTriggerService.cs:192
- Same issue as line 162 - the URL is missing the base URL prefix. It should use
$"{_adoBaseUrl}/{adoOrg.EscapeDataString()}..."
to match the pattern used in other API calls.
{
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
JTokenType.Bytes => throw new NotImplementedException(), | ||
JTokenType.Guid => throw new NotImplementedException(), | ||
JTokenType.Uri => throw new NotImplementedException(), | ||
JTokenType.TimeSpan => throw new NotImplementedException(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using 'throw new NotImplementedException()' for multiple JTokenType cases is not ideal. Consider throwing a more descriptive exception like ArgumentException or InvalidOperationException with a meaningful message about the unsupported token type.
JTokenType.TimeSpan => throw new NotImplementedException(), | |
JTokenType.None => throw new InvalidOperationException($"Unsupported JTokenType: None for reportBuildStatus property."), | |
JTokenType.Object => throw new InvalidOperationException($"Unsupported JTokenType: Object for reportBuildStatus property."), | |
JTokenType.Array => throw new InvalidOperationException($"Unsupported JTokenType: Array for reportBuildStatus property."), | |
JTokenType.Constructor => throw new InvalidOperationException($"Unsupported JTokenType: Constructor for reportBuildStatus property."), | |
JTokenType.Property => throw new InvalidOperationException($"Unsupported JTokenType: Property for reportBuildStatus property."), | |
JTokenType.Comment => throw new InvalidOperationException($"Unsupported JTokenType: Comment for reportBuildStatus property."), | |
JTokenType.Integer => throw new InvalidOperationException($"Unsupported JTokenType: Integer for reportBuildStatus property."), | |
JTokenType.Float => throw new InvalidOperationException($"Unsupported JTokenType: Float for reportBuildStatus property."), | |
JTokenType.Null => throw new InvalidOperationException($"Unsupported JTokenType: Null for reportBuildStatus property."), | |
JTokenType.Undefined => throw new InvalidOperationException($"Unsupported JTokenType: Undefined for reportBuildStatus property."), | |
JTokenType.Date => throw new InvalidOperationException($"Unsupported JTokenType: Date for reportBuildStatus property."), | |
JTokenType.Raw => throw new InvalidOperationException($"Unsupported JTokenType: Raw for reportBuildStatus property."), | |
JTokenType.Bytes => throw new InvalidOperationException($"Unsupported JTokenType: Bytes for reportBuildStatus property."), | |
JTokenType.Guid => throw new InvalidOperationException($"Unsupported JTokenType: Guid for reportBuildStatus property."), | |
JTokenType.Uri => throw new InvalidOperationException($"Unsupported JTokenType: Uri for reportBuildStatus property."), | |
JTokenType.TimeSpan => throw new InvalidOperationException($"Unsupported JTokenType: TimeSpan for reportBuildStatus property."), |
Copilot uses AI. Check for mistakes.
}.ToJson(); | ||
|
||
var repoUrl = $"/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0"; | ||
var policyUrl = $"/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId={repositoryId}&api-version=6.0"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test is using an incomplete URL pattern that matches the bug in the production code. This should include the full base URL to properly test the actual API call pattern.
var policyUrl = $"/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId={repositoryId}&api-version=6.0"; | |
var repoUrl = $"https://dev.azure.com/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/git/repositories/{REPO_NAME.EscapeDataString()}?api-version=6.0"; | |
var policyUrl = $"https://dev.azure.com/{ADO_ORG.EscapeDataString()}/{TEAM_PROJECT.EscapeDataString()}/_apis/policy/configurations?repositoryId={repositoryId}&api-version=6.0"; |
Copilot uses AI. Check for mistakes.
Unit Test Results0 tests 0 ✅ 0s ⏱️ Results for commit c945151. ♻️ This comment has been updated with latest results. |
Need to finish #1412 first |
A comprehensive test mode for the
gh ado2gh rewire-pipeline
command that covers the scenarios below:Overview
The pipeline testing feature allows to validate Azure DevOps pipelines before permanently migrating them to GitHub. It provides two testing modes:
--dry-run
): Test a specific pipelinetest-pipelines
command): Test multiple pipelines concurrentlySingle Pipeline Testing
Usage
Parameters
--dry-run
: Enables test mode (temporarily rewires, tests, then restores)--monitor-timeout-minutes
: How long to wait for build completion (default: 30 minutes)Process Flow
Sample Output
Batch Pipeline Testing
Usage
Parameters
--pipeline-filter
: Wildcard pattern to filter pipelines (e.g., "CI-", "-Deploy", "*")--max-concurrent-tests
: Maximum number of concurrent tests (default: 3)--report-path
: Path to save detailed JSON report (default: pipeline-test-report.json)--monitor-timeout-minutes
: Timeout for each pipeline test (default: 30 minutes)Process Flow
Sample Output
Report Structure
JSON Report Format
Error Handling and Recovery
Automatic Recovery
The testing system is designed to be resilient:
Manual Recovery
If automatic restoration fails:
Best Practices
Before Testing
During Testing
After Testing
Troubleshooting
Common Issues
Build Failures
Restoration Failures
Timeout Issues
--monitor-timeout-minutes
for longer buildsSupport Information
For issues or questions about pipeline testing: