Skip to content

RunTests action - #2343

Open
spetersenms wants to merge 57 commits into
microsoft:mainfrom
spetersenms:spetersen/separateTestAction
Open

spetersenms wants to merge 57 commits into
microsoft:mainfrom
spetersenms:spetersen/separateTestAction

Conversation

@spetersenms

@spetersenms spetersenms commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds the preview useSeparateTestAction setting, defaulting to false, which moves normal test execution from RunPipeline into a new RunTests action using al runtests.

Flow

  1. RunPipeline builds and publishes the apps, creates the container, and keeps it alive.
  2. It exports the container name, generated credentials, and a runtime signal.
  3. The workflow conditionally invokes RunTests.
  4. RunTests selects the test apps, executes them with AlTool, and produces JUnit results for the existing analysis step.
  5. The existing cleanup step removes the container.

Notes

  • Only normal tests are moved. BCPT, page scripting, and multi-country tests remain in RunPipeline.
  • runTestsInAllInstalledTestApps uses the existing installTestAppsJson handoff rather than discovering every installed server app.
  • The optional testType setting supports UnitTest, IntegrationTest, and Uncategorized.
  • A custom RunTestsInBcContainer override can still replace the built-in AlTool runner.
  • BCH is still used for container creation, app installation and metadata, server and company discovery, test enumeration, and event-log collection. AlTool owns the actual test execution.

✅ Checklist

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

spetersenms and others added 2 commits August 6, 2026 16:32
Introduces a new opt-in RunTests action that moves normal test execution
(testFolders) out of the RunPipeline action. When useSeparateTestAction is
true, RunPipeline compiles, publishes and installs apps and keeps the build
container alive, but skips normal tests; the RunTests action then runs those
tests against the same container and produces TestResults.xml. BCPT and page
scripting tests are unchanged. Behavior is unchanged when the setting is false.

Includes the new setting (ReadSettings, settings.schema.json, settings.md),
the RunPipeline keep-alive guard, template _BuildALGoProject wiring, release
notes and Pester tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment thread Actions/RunTests/RunTests.ps1 Fixed
spetersenms and others added 23 commits August 7, 2026 10:04
The separate RunTests action now runs normal tests through Microsoft's
headless al runtests (AlTool) CLI by default instead of BcContainerHelper.
The runner resolves the kept-alive container's connection settings host-side
from the container name, installs the AL developer tools as a dotnet global
tool, runs the test codeunits batched (with an isolated fallback/rerun pass)
and emits the same TestResults.xml (JUnit) schema, so downstream test result
analysis is unchanged. No RunPipeline change is required.

BcContainerHelper remains available via a RunTestsInBcContainer override
script, which fully replaces the built-in AlTool runner (this is how, for
example, BCApps supplies its own runner and how Legacy test-type tests are
handled). The AlTool runner does not run Legacy test types or tests that
require UI/client-callback interaction; this is documented as a known
limitation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The official released al runtests tool does not support --testplan, so the
batch-first path in AlToolTestRunner hard-set �llPassed=false whenever the
batch failed to connect and never reset it, making every run falsely report
FAILED. Remove the batching path entirely and run each test codeunit in its
own �l runtests <codeunitId> --testmethods --raw invocation, with the
existing rerun-on-failure pass. �llPassed now derives solely from real
per-method outcomes via Add-JUnitTestSuite.

- Delete Invoke-AlBatchRunTests, ConvertFrom-AlBatchOutput,
  ConvertTo-AlTestPlanJson and their Export-ModuleMember entries.
- Update README and RELEASENOTES to per-codeunit wording.
- Drop the two batch-specific unit test contexts.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@spetersenms
spetersenms marked this pull request as ready for review August 25, 2026 07:16
@spetersenms
spetersenms requested a review from a team as a code owner August 25, 2026 07:16
Copilot AI balanced review requested due to automatic review settings August 25, 2026 07:16
@mazhelez

Copy link
Copy Markdown
Collaborator

Copilot resolve the merge conflicts in this pull request

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comment thread Actions/RunTests/AlToolTestRunner.psm1 Outdated
Comment thread Tests/AL-Go-Helper.Test.ps1
Comment thread Actions/RunTests/RunTests.psm1 Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Separate execution does not preserve test-disable settings, test-data preparation, and configured database isolation.

0 open findings

3 resolved since last review
Previously missed (3)

In code that hasn't changed since last review

Medium severity Exclude doNotBuildTests from RunTests eligibility

Actions/​RunPipeline/​RunPipeline.ps1:526

The eligibility check still enables RunTests when doNotBuildTests is true and doNotRunTests is false. BcContainerHelper disables test execution internally for doNotBuildTests, but that does not update $settings or this workflow signal. The separate runner can therefore select downloaded test apps or existing compiled test artifacts and attempt tests that were explicitly disabled, potentially without the test toolkit installed. Exclude doNotBuildTests from eligibility and add a regression case for this combination.

Medium severity Preserve test-data import when all test categories are disabled

Actions/​RunPipeline/​RunPipeline.ps1:534

When doNotRunBcptTests and doNotRunPageScriptingTests are both true, setting doNotRunTests here also skips BcContainerHelper's ImportTestDataInBcContainer stage: that stage requires at least one enabled test category. Configured configPackages and custom test-data imports are then omitted, and RunTests invokes the runner without performing that preparation. Preserve the import stage before separate test execution, or retain the existing pipeline path for builds that need it.

Medium severity Honor BeforeEachTestApp database restoration in separate runner

Actions/​RunTests/​RunTests.psm1:314

This loop does not honor restoreDatabases: ["BeforeEachTestApp"]. RunPipeline still passes that setting to BcContainerHelper, but normal tests are now skipped there, so its per-app restore never runs. The separate runner executes each app against state left by earlier apps or by BCPT/page scripting, violating the configured isolation. Preserve the baseline backup and restore it before each normal test app, or leave builds using this option on the existing RunPipeline path.

🧠 Review effort: Balanced

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

A few more file to review.

Comment thread Actions/RunTests/README.md Outdated
Comment thread Actions/RunTests/README.md Outdated
Comment thread Actions/RunTests/AlToolTestRunner.psm1 Outdated
… around parsing callstack for failed AlTool test runs.

SetTokenAndRepository -github:$github -githubOwner $githubOwner -appId $e2eAppId -appKey $e2eAppKey -repository $repository
if (Test-E2ERepositoryExists -repository $repository) {
throw "Refusing to use existing repository $repository for the E2E scenario."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is this needed?


$summary, $failures, $failureSummary = GetTestResultSummaryMD -testResultsFile $junitFile

$summary | Should -Match '\|Test App\|3\|1[^|]*\|1[^|]*\|1[^|]*\|0\.03\|'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not sure what this is testing.

Comment thread Actions/AL-Go-Helper.ps1
.OUTPUTS
[pscustomobject] with StandardOutput, StandardError, combined Output, and ExitCode properties.
#>
function Invoke-AlNativeCommand {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How about moving the al-related calling out of AL-Go helper into its own module?

This branch has not been deployed

No deployments
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.

5 participants