Skip to content

Refactor FederatedCredentials e2e test to use stable repository with config-based disabling#2104

Open
Copilot wants to merge 35 commits intomainfrom
copilot/refactor-federated-credentials-test
Open

Refactor FederatedCredentials e2e test to use stable repository with config-based disabling#2104
Copilot wants to merge 35 commits intomainfrom
copilot/refactor-federated-credentials-test

Conversation

Copy link
Contributor

Copilot AI commented Jan 29, 2026

❔What, Why & How

Federated credentials require stable repository identity. The FederatedCredentials e2e test was deleting/recreating tmp-bingmaps.appsource on each run, breaking federated credential authentication.

This PR refactors the test to reuse a pre-existing e2e-bingmaps.appsource repository instead of deleting and recreating it, and implements a config-based mechanism for disabling test scenarios.

Helper functions added to e2eTestHelper.psm1:

  • CleanupWorkflowRuns: Deletes all workflow runs in a repository with pagination support (ensures clean tracking state)
  • ResetRepositoryToSource: Hard resets repository content to match source via git reset --hard + force push (preserves identity, resets content) with individual error handling for each git operation

Test flow changes in scenarios/FederatedCredentials/runtest.ps1:

  • Renamed repository: tmp-bingmaps.appsourcee2e-bingmaps.appsource (avoids tmp* cleanup pattern)
  • Requires pre-existing repository with federated credentials configured (errors if missing)
  • Resets to bcsamples-bingmaps.appsource content before each run
  • Captures previous workflow run IDs BEFORE cleanup (fixed logic issue where cleanup deleted all runs before capturing IDs)
  • Cleans all workflow runs to ensure clean state
  • Re-applies test settings (ghTokenWorkflowSecretName) after reset
  • Adopted established polling pattern from CommitAndPush function for workflow run tracking (replaces timestamp-based approach):
    • Captures previous run IDs before any pushes (including settings push)
    • Polls with 10-second intervals until new run appears
    • Handles race condition where settings push triggers intermediate workflow run
    • Maximum 60 attempts with timeout error

Workflow changes in .github/workflows/E2E.yaml:

  • Created e2eTests/disabled-scenarios.json config file to manage disabled scenarios (array format with scenario and reason in each object)
  • Loads disabled scenarios from config with error handling for missing/empty files
  • Filters disabled scenarios from test matrix entirely (most efficient approach)
  • Comprehensive logging shows which scenarios are disabled, filtering details with reasons, and final scenario list

Technical improvements:

  • Mandatory parameters on helper functions
  • GUID-based temp paths to avoid conflicts
  • Removed unnecessary LASTEXITCODE checks (invoke-gh/invoke-git wrappers throw exceptions via cmdDo)
  • Removed unsupported -- --quiet flag from gh repo clone
  • Individual try-catch blocks for each git operation with specific error messages
  • Config-driven filtering prevents disabled scenarios from being included in matrix
  • Error handling for missing config file (gracefully proceeds with all scenarios enabled)
  • Uses invoke-gh wrapper consistently for API calls with try-catch error handling
  • Removed unused variables and duplicate null checks
  • Fixed typos in comments

To re-enable FederatedCredentials test: Remove the FederatedCredentials object from the array in e2eTests/disabled-scenarios.json.

When re-enabled, the test will use the stable repository instead of recreating it, maintaining federated credential authentication.

✅ Checklist

  • Add tests (E2E, unit tests)
  • Update RELEASENOTES.md
  • Update documentation (e.g. for new settings or scenarios)
  • Add telemetry
Original prompt

Refactor the FederatedCredentials end-to-end test scenario in microsoft/AL-Go so it reuses an existing temporary repository instead of deleting and recreating it on every run. The current approach creates a new repository called tmp-bingmaps.appsource copied from microsoft/bcsamples-bingmaps.appsource. Federated credentials no longer work with repository name-based matching, so the repository must be stable across runs.

Context:

  • Failing job URL: https://github.com/microsoft/AL-Go/actions/runs/21470716621/job/61842467415
  • Failure occurs when the test triggers the "Update AL-Go System Files" workflow in the newly created repository.
  • The test script is at e2eTests/scenarios/FederatedCredentials/runtest.ps1 and currently:
    • checks if the temp repo exists
    • deletes it if it exists
    • recreates it from template
    • then runs Update AL-Go System Files

Required changes:

  1. Modify e2eTests/scenarios/FederatedCredentials/runtest.ps1 to reuse the repo if it already exists.
    • Do not delete the repo anymore.
    • If the repo exists, reset it to match the source repo (microsoft/bcsamples-bingmaps.appsource) in a deterministic way.
    • Ensure the repo ends up with the same default branch content expected by the scenario (main).
  2. Add cleanup logic for workflow runs in the reused temp repo to prevent the run list from growing and to ensure the subsequent WaitWorkflow logic targets the correct run.
    • Before starting the scenario, delete older workflow runs (keep a small number like the last 5-10), or alternatively capture the run id of the workflow triggered and wait specifically for that.
  3. Ensure secrets and settings required for the test are still present/updated.
    • The script sets Azure_Credentials; preserve this.
    • The script passes addRepoSettings for ghTokenWorkflowSecretName; ensure this still works if repo already exists.
  4. Update any helper functions or supporting scripts if needed to support resetting repo contents and cleaning workflow runs.

Acceptance criteria:

  • The test no longer relies on deleting/recreating tmp-bingmaps.appsource.
  • The temp repo is stable across runs and compatible with federated credentials.
  • The script reliably waits for and inspects the workflow run it triggered, not an older run.
  • Add clear comments explaining why the repo is reused.

Relevant files:

  • e2eTests/scenarios/FederatedCredentials/runtest.ps1
  • e2eTests/Workflows/RunUpdateAlGoSystemFiles.ps1 (workflow trigger helper)
  • Any relevant helper module under e2eTests/e2eTestHelper.psm1 if needed.

This pull request was created from Copilot chat.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 3 commits January 29, 2026 10:51
…repository

Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
…rove pagination and timestamp comparison

Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor FederatedCredentials test to reuse temporary repository Refactor FederatedCredentials test to reuse temporary repository Jan 29, 2026
Copilot AI requested a review from mazhelez January 29, 2026 10:59
…federated credentials

Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
Copilot AI changed the title Refactor FederatedCredentials test to reuse temporary repository Refactor FederatedCredentials test to require pre-existing repository with federated credentials Jan 29, 2026
Copilot AI requested a review from mazhelez January 29, 2026 12:03
Copy link
Collaborator

@mazhelez mazhelez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work!

@copilot the following script is used in a workflow to clean up temp repositories: "Internal\Scripts\RemoveTempRepos.ps1"
Can you adjust the script or rename the repository, so that it doesn't get deleted?

…function, exclude tmp-bingmaps from cleanup

Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jan 29, 2026

Good work!

@copilot the following script is used in a workflow to clean up temp repositories: "Internal\Scripts\RemoveTempRepos.ps1"...

Done. Updated Internal/Scripts/RemoveTempRepos.ps1 to exclude tmp-bingmaps.appsource from automatic deletion. The repository will now remain stable across cleanup operations. Commit: 2309402

Copilot AI requested a review from mazhelez January 29, 2026 12:24
…ce to avoid auto-cleanup

Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
Copilot AI changed the title Refactor FederatedCredentials test to require pre-existing repository with federated credentials Refactor FederatedCredentials test to use stable e2e-bingmaps.appsource repository Jan 29, 2026
Copilot AI requested a review from mazhelez January 29, 2026 12:29
mazhelez and others added 2 commits February 6, 2026 10:28
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…cs, add null checks, improve config validation

Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
…ps; handle settings push race condition

Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
mazhelez and others added 2 commits February 6, 2026 11:13
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@mazhelez
Copy link
Collaborator

mazhelez commented Feb 6, 2026

@copilot can you please remove the trailing whitespace in the modified files

Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 6, 2026

@copilot can you please remove the trailing whitespace in the modified files

Done. Removed trailing whitespace from runtest.ps1 (lines 146, 149, 150, 152, 159). Commit: 13bcc5e

Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +105 to +160
# Re-apply the custom repository settings that were lost during reset
$tempPath = [System.IO.Path]::GetTempPath()
$repoPath = Join-Path $tempPath ([System.Guid]::NewGuid().ToString())
New-Item $repoPath -ItemType Directory | Out-Null
Push-Location $repoPath
try {
Write-Host "Re-applying repository settings..."
invoke-gh repo clone $repository .
$repoSettingsFile = ".github\AL-Go-Settings.json"
if (Test-Path $repoSettingsFile) {
Add-PropertiesToJsonFile -path $repoSettingsFile -properties @{"ghTokenWorkflowSecretName" = "e2eghTokenWorkflow"}
invoke-git add $repoSettingsFile
invoke-git commit -m "Update repository settings for test" --quiet
invoke-git push --quiet
Write-Host "Settings push completed. This will trigger a CI/CD workflow run."
}
else {
Write-Host "Warning: .github\AL-Go-Settings.json not found after cloning. Settings may not be applied correctly."
}
}
finally {
Pop-Location
Remove-Item -Path $repoPath -Force -Recurse -ErrorAction SilentlyContinue
}

# Upgrade AL-Go System Files to test version
RunUpdateAlGoSystemFiles -directCommit -wait -templateUrl $template -repository $repository | Out-Null
RunUpdateAlGoSystemFiles -directCommit -wait -repository $repository | Out-Null

# Wait for CI/CD workflow to start (triggered by Update AL-Go System Files)
# This follows the established pattern from CommitAndPush: poll until a new run appears
Write-Host "Waiting for CI/CD workflow to start (triggered by Update AL-Go System Files push)..."

# Poll for new workflow run that wasn't in the previous list
$maxAttempts = 60 # 10 minutes maximum wait (60 * 10 seconds)
$attempts = 0
$run = $null

while ($attempts -lt $maxAttempts) {
Start-Sleep -Seconds 10
$attempts++

$currentRuns = invoke-gh api /repos/$repository/actions/runs -silent -returnValue | ConvertFrom-Json
# Find new push workflow runs that weren't in our previous list
$newRuns = $currentRuns.workflow_runs | Where-Object {
$_.event -eq 'push' -and $previousRunIds -notcontains $_.id
}

if ($newRuns) {
# Get the most recent new run (first one, since API returns newest first)
$run = $newRuns | Select-Object -First 1
Write-Host "Found new CI/CD workflow run: $($run.id) (created at $($run.created_at))"
break
}

Write-Host "Waiting for new CI/CD workflow run to appear (attempt $attempts/$maxAttempts)..."
}
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manual push at lines 106-128 triggers a CI/CD workflow run, but the polling logic starting at line 133 waits for workflow runs triggered by RunUpdateAlGoSystemFiles at line 131. This creates a race condition where the poller might pick up the workflow run from the manual settings push instead of the one from RunUpdateAlGoSystemFiles. Consider capturing workflow run IDs after the manual settings push completes and before calling RunUpdateAlGoSystemFiles to ensure proper tracking.

Copilot uses AI. Check for mistakes.
WaitWorkflow -repository $repository -runid $run.id -noError

# The CI/CD workflow should fail because the version number of the app in thie repository is lower than the version number in AppSource
# The CI/CD workflow should fail because the version number of the app in the repository is lower than the version number in AppSource
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says "thie repository" but it should be "the repository" (typo: "thie" → "the").

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already fixed in commit b8d28e8. The comment originally said "tmp repo" (referring to the old repository name) and was updated to "e2e-bingmaps.appsource repo" to reflect the new repository name.

…e duplicate null check

Co-authored-by: mazhelez <43066499+mazhelez@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants