Prototype: Standard/Evaluation DemoTool smoke test in Legacy CI

[aholstrup1]

What & why

BCApps legacy tests only ever generate Extended demo data. The Standard/Evaluation generation path (Interface Trial Data.CreateSetupData, CU122000, selected in Create Demonstration Data CU101900) is never exercised in BCApps CI. Only NAV's BuildTranslatedCountryDatabase.ps1 loops Standard/Evaluation/Extended when building shippable packages. As a result, a class of DemoTool runtime bugs escapes BCApps and only fails later in the NAV repo (which consumes BCApps as a submodule).

Concrete example: sync PR #8928 accidentally deleted src/Layers/NL/DemoTool/CreateCurrency.Codeunit.al. BCApps stayed green; the NAV Standard build then failed with "The field Realized Exchange Gains Account of table Currency contains a value (9620) not found in G/L Account". Fixed by re-adding the override in #9135, but the coverage gap remained.

This adds an opt-in smoke test that runs the legacy DemoTool for the requested non-Extended data types against throwaway companies, gated by a new smokeTestDemoDataTypes AL-Go setting. It runs in the Legacy branch of Invoke-DemoDataGeneration, before New-TestCompany rebuilds the real Extended test company from a clean slate, so it cannot affect the demo data the tests actually run against. The plumbing was mostly already there: Initialize-DemoToolResources already accepts -DemoDataType and writes it into the staged config.

Enabled on Test Apps NL (Standard + Evaluation) as the proving ground, so between the smoke test and the real Extended run, all three data types get exercised for NL.

Warning

Draft / not for merge as-is. The second commit deliberately reverts the #9135 fix (deletes CreateCurrency.Codeunit.al) to reintroduce the 9620 bug and prove the smoke test catches it in CI. That revert must be dropped before this could merge.

Linked work

N/A (prototype / validation experiment). Related: AB#641606, root-cause fix #9135 / AB#641121.

Fixes #

How I validated this

• I read the full diff and it contains only changes I intended.
• I built the affected app(s) locally with no new analyzer warnings.
• I ran the change in Business Central and confirmed it behaves as expected.
• I added or updated tests for the new behavior, or explained below why none are needed.

What I tested and the outcome

• PowerShell parses cleanly ([System.Management.Automation.Language.Parser]::ParseFile -> no syntax errors); NL settings.json validated as JSON.
• Full container/CI validation is exactly what this draft is for: with the reintroduced bug (commit 2), the NL Legacy Bucket1 job's new Standard/Evaluation smoke test is expected to fail with the account-9620 error, demonstrating it catches what Extended-only CI missed. With the fix intact it should pass.
• No AL test objects added; this is CI-tooling only.

Risk & compatibility

• Fully opt-in: when smokeTestDemoDataTypes is unset/empty, behavior is unchanged. Only enabled on Test Apps NL here.
• Isolation: the smoke test uses disposable companies created and removed before New-TestCompany, which then wipes all companies and rebuilds the real one, so the real Extended test data is provably untouched.
• CI cost: one extra demo-data generation per participating Legacy job (Legacy jobs are the slowest). Scoped to Bucket1 only, and to localizations if rolled out further.
• Unknowns: first-run shakeout of Standard/Evaluation generation inside a container (rapidstart export to user folders, language packs, tenant-ready timing).
• Contains a deliberate revert of re-added createcurrency for NL #9135 for validation; must be removed before merge.

[github-actions]

Issue #9135 is not valid. Please make sure you link an issue that exists, is open and is approved.

[github-actions]

$\textbf{🟡\ Medium\ Severity\ —\ Agent} \quad \color{gray}{\texttt{\small Iteration\ 1}}$

In the new Invoke-AdditionalLegacyDemoData function, the catch block throws a carefully constructed diagnostic message (including DataType and country context) specifically to make this class of DemoTool failure easier to triage.

However, the finally block unconditionally attempts Remove-CompanyInBcContainer for the throwaway company. In PowerShell (as in .NET generally), an exception raised inside a finally block replaces/masks any exception already propagating from the try/catch above it. Because the scenario this function is built to catch is exactly a failure during demo-data generation into that company, the company/container state is plausibly left in a condition where the cleanup Remove-CompanyInBcContainer call itself fails or times out -- and if it does, the informative error this function exists to produce is silently discarded, replaced by an unrelated cleanup-failure message.

Recommendation:

• capture the original error in the catch block, wrap the cleanup call in its own try/catch that only logs (does not throw) on cleanup failure, and re-throw the original diagnostic error afterward so cleanup problems never suppress the primary failure.

_{👍 useful · ❤️ especially valuable · 👎 wrong - reply with why}

[github-actions]

Copilot PR Review

Iteration 1 · Outcome: completed

PR changes are PowerShell build tooling (ImportTestDataInBcContainer.ps1) and AL-Go JSON settings, not AL source. No AL leaf skill found applicable knowledge-domain matches; the super-skill self-review pass surfaced one cross-cutting reliability concern.

Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630

Findings by domain

Findings split into Knowledge-backed (cite a BCQuality article) and Agent (the agent's own judgement, no matching BCQuality rule).

Domain	Findings	Knowledge-backed	Agent	Inline	Fallback
Agent	1	0	1	1	0

Totals: 0 knowledge-backed · 1 agent findings.

Orchestrator pre-filter (13 file(s) excluded)

• layer-disabled (knowledge) : 13 file(s)

_{Findings produced by the Copilot CLI agent against BCQuality at 822cae1b2771ac25f665f73369f69093bd4fd630. Reply 👎 on any inline comment to flag false positives.}