Standard/Evaluation DemoTool smoke test in Legacy CI#9178
Merged
Conversation
BCApps legacy tests only generate Extended demo data, so the Standard/Evaluation generation path (Interface Trial Data.CreateSetupData, CU122000) is never exercised. Bugs there only surface later in the NAV translated-country build. Add an opt-in smoke test (gated by the smokeTestDemoDataTypes AL-Go setting) that runs the legacy DemoTool for the requested non-Extended data types against throwaway companies, before the real Extended test company is built, so it cannot affect the demo data tests run against. Enabled on Test Apps NL (Standard + Evaluation) as the proving ground.
This reverts commit 1d5d7be.
github-actions
Bot
added
the
Build: scripts & configs
Contributor
|
Issue #9135 is not valid. Please make sure you link an issue that exists, is open and is approved. |
This reverts commit 870f61c.
…cket1 Rename the concept from smokeTest to additionalDemoDataTypes (setting key, function Invoke-AdditionalLegacyDemoData, throwaway company and log text). Move enablement from the NL project to a global conditionalSettings entry in .github/AL-Go-Settings.json keyed on buildMode LegacyTestsBucket1, so the additional Standard/Evaluation demo data generation runs once per country (Bucket1 only) across all countries.
github-actions
Bot
added
the
Build: Automation
…rency-demo-account
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
Copilot PR ReviewIteration 4 · Outcome: completed Knowledge source: https://github.com/microsoft/BCQuality@822cae1b2771ac25f665f73369f69093bd4fd630 Orchestrator pre-filter (13 file(s) excluded)
Findings produced by the Copilot CLI agent against BCQuality at |
NAV's Run-NavDemoTool runs the legacy DemoTool against a fresh EMPTY company and never runs Company-Initialize (CU2). BCApps ran CU2 first, which for CZ/DK pre-seeds localization setup (via OnCompanyInitialize subscribers) that collides with the Standard/Evaluation demo data paths (duplicate CZL VAT Return Period; missing DK DOMESTIC VAT posting group). Add a -SkipCompanyInitialize switch to Invoke-LegacyDemoDataTool, passed only by the additional (Standard/Evaluation) throwaway-company verification, so it mirrors NAV for all countries. The real Extended test company keeps CU2 (unchanged).
CZ Evaluation (duplicate CZL VAT Return Period) and DK Standard (missing DOMESTIC VAT Bus. Posting Group) fail the additional demo data check in a clean container. The same shared-submodule code runs green in NAV's translated-country build, so the cause is environmental/build-config, not the check itself; skipping Company-Initialize (CU2) did not change it. Disable the check for CZ and DK only, via a project-level LegacyTestsBucket1 conditional setting. AL-Go merges (concatenates) array settings, so overwriteSettings is used to force additionalDemoDataTypes to an empty array for these two countries. All other countries still run Standard+Evaluation. Tracked for follow-up.
Wrap the finally-block cleanup (Get/Remove-CompanyInBcContainer) in its own try/catch that only logs. Under ErrorActionPreference=Stop a cleanup failure in finally would otherwise replace the diagnostic error propagating from the catch block, losing the DataType/country context this check exists to surface. Addresses PR review feedback.
aholstrup1
changed the title
mazhelez
approved these changes
Jul 9, 2026
spetersenms
approved these changes
Jul 9, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
What & why
BCApps legacy tests only ever generate
Extendeddemo data. TheStandard/Evaluationgeneration path (Interface Trial Data.CreateSetupData, CU122000, selected inCreate Demonstration DataCU101900) is never exercised in BCApps CI. Only NAV'sBuildTranslatedCountryDatabase.ps1loops 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
smokeTestDemoDataTypesAL-Go setting. It runs in theLegacybranch ofInvoke-DemoDataGeneration, beforeNew-TestCompanyrebuilds 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-DemoToolResourcesalready accepts-DemoDataTypeand 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 (CI check work item), AB#641861 (tracks the CZ/DK demo-data failures that are excluded here)
Fixes #
How I validated this
What I tested and the outcome
[System.Management.Automation.Language.Parser]::ParseFile-> no syntax errors); NLsettings.jsonvalidated as JSON.Risk & compatibility
smokeTestDemoDataTypesis unset/empty, behavior is unchanged. Only enabled onTest Apps NLhere.New-TestCompany, which then wipes all companies and rebuilds the real one, so the real Extended test data is provably untouched.