-
Notifications
You must be signed in to change notification settings - Fork 250
Fix: Use target repository for git operations in create-pull-request #15501
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
Merged
+222
−21
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
147 changes: 147 additions & 0 deletions
147
pkg/workflow/create_pull_request_cross_repo_integration_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| //go:build integration | ||
|
|
||
| package workflow | ||
|
|
||
| import ( | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestCreatePullRequestCrossRepoCheckout tests that target-repo properly configures checkout and git | ||
| func TestCreatePullRequestCrossRepoCheckout(t *testing.T) { | ||
| tmpDir, err := os.MkdirTemp("", "cross-repo-checkout-test") | ||
| require.NoError(t, err, "Failed to create temp dir") | ||
| defer os.RemoveAll(tmpDir) | ||
|
|
||
| // Create test workflow with cross-repo target | ||
| workflowContent := `--- | ||
| on: push | ||
| permissions: | ||
| contents: read | ||
| actions: read | ||
| issues: read | ||
| pull-requests: read | ||
| engine: copilot | ||
| safe-outputs: | ||
| create-pull-request: | ||
| target-repo: "microsoft/vscode-docs" | ||
| base-branch: vnext | ||
| draft: true | ||
| --- | ||
|
|
||
| # Cross-Repo Test Workflow | ||
|
|
||
| Create a pull request in a different repository. | ||
| ` | ||
|
|
||
| workflowPath := filepath.Join(tmpDir, "cross-repo.md") | ||
| require.NoError(t, os.WriteFile(workflowPath, []byte(workflowContent), 0644), "Failed to write workflow file") | ||
|
|
||
| // Compile the workflow | ||
| compiler := NewCompiler() | ||
| require.NoError(t, compiler.CompileWorkflow(workflowPath), "Failed to compile workflow") | ||
|
|
||
| // Read the compiled output | ||
| outputFile := filepath.Join(tmpDir, "cross-repo.lock.yml") | ||
| compiledBytes, err := os.ReadFile(outputFile) | ||
| require.NoError(t, err, "Failed to read compiled output") | ||
|
|
||
| compiledContent := string(compiledBytes) | ||
|
|
||
| // Test 1: Verify repository parameter is set in actions/checkout | ||
| assert.Contains(t, compiledContent, "repository: microsoft/vscode-docs", | ||
| "Expected checkout to specify target repository") | ||
|
|
||
| // Test 2: Verify REPO_NAME environment variable is set to target repo | ||
| assert.Contains(t, compiledContent, `REPO_NAME: "microsoft/vscode-docs"`, | ||
| "Expected REPO_NAME env var to be set to target repository") | ||
|
|
||
| // Test 3: Verify token is included for cross-repo checkout | ||
| assert.Contains(t, compiledContent, "token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }}", | ||
| "Expected token to be set for cross-repo checkout") | ||
|
|
||
| // Test 4: Verify it does NOT use the default github.repository | ||
| checkoutSection := extractCheckoutSection(compiledContent) | ||
| assert.NotContains(t, checkoutSection, "github.repository", | ||
| "Checkout section should not reference github.repository when target-repo is set") | ||
| } | ||
|
|
||
| // TestCreatePullRequestSameRepoCheckout tests that without target-repo, we use default checkout | ||
| func TestCreatePullRequestSameRepoCheckout(t *testing.T) { | ||
| tmpDir, err := os.MkdirTemp("", "same-repo-checkout-test") | ||
| require.NoError(t, err, "Failed to create temp dir") | ||
| defer os.RemoveAll(tmpDir) | ||
|
|
||
| // Create test workflow without target-repo | ||
| workflowContent := `--- | ||
| on: push | ||
| permissions: | ||
| contents: read | ||
| actions: read | ||
| issues: read | ||
| pull-requests: read | ||
| engine: copilot | ||
| safe-outputs: | ||
| create-pull-request: | ||
| draft: true | ||
| --- | ||
|
|
||
| # Same-Repo Test Workflow | ||
|
|
||
| Create a pull request in the same repository. | ||
| ` | ||
|
|
||
| workflowPath := filepath.Join(tmpDir, "same-repo.md") | ||
| require.NoError(t, os.WriteFile(workflowPath, []byte(workflowContent), 0644), "Failed to write workflow file") | ||
|
|
||
| // Compile the workflow | ||
| compiler := NewCompiler() | ||
| require.NoError(t, compiler.CompileWorkflow(workflowPath), "Failed to compile workflow") | ||
|
|
||
| // Read the compiled output | ||
| outputFile := filepath.Join(tmpDir, "same-repo.lock.yml") | ||
| compiledBytes, err := os.ReadFile(outputFile) | ||
| require.NoError(t, err, "Failed to read compiled output") | ||
|
|
||
| compiledContent := string(compiledBytes) | ||
|
|
||
| // Test 1: Verify no explicit repository parameter (uses default) | ||
| checkoutSection := extractCheckoutSection(compiledContent) | ||
| assert.NotContains(t, checkoutSection, "repository:", | ||
| "Checkout section should not have explicit repository when using source repo") | ||
|
|
||
| // Test 2: Verify REPO_NAME uses github.repository expression | ||
| assert.Contains(t, compiledContent, "REPO_NAME: ${{ github.repository }}", | ||
| "Expected REPO_NAME to use github.repository expression for same-repo") | ||
|
|
||
| // Test 3: Verify no token in checkout (not needed for same repo) | ||
| assert.NotContains(t, checkoutSection, "token:", | ||
| "Checkout section should not have token for same-repo checkout") | ||
| } | ||
|
|
||
| // extractCheckoutSection extracts the checkout step from compiled YAML for inspection | ||
| func extractCheckoutSection(content string) string { | ||
| lines := strings.Split(content, "\n") | ||
| inCheckout := false | ||
| var checkoutLines []string | ||
|
|
||
| for _, line := range lines { | ||
| if strings.Contains(line, "name: Checkout repository") { | ||
| inCheckout = true | ||
| } | ||
| if inCheckout { | ||
| checkoutLines = append(checkoutLines, line) | ||
| // Stop at the next step (less indentation than " -") | ||
| if strings.HasPrefix(line, " - name:") && !strings.Contains(line, "Checkout repository") { | ||
| break | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return strings.Join(checkoutLines, "\n") | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
The condition for adding a token should consider effectiveTargetRepo instead of just targetRepoSlug. If trialLogicalRepoSlug is set (in trial mode), a token is already included, but the current logic may not include a token when only targetRepoSlug is set without trial mode.
However, after the priority bug is fixed, this condition should be:
if effectiveTargetRepo != ""to properly handle all cases where we're checking out a different repository.This issue also appears on line 36 of the same file.