Conversation
The ModifyBuildWorkflows function used by workflow sanitation tests was not accounting for the new MergeCoverage job when building the PostProcess job's needs array. This caused CICD.yaml comparison to fail. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Guard optional hashtable property accesses that throw PropertyNotFoundException under Set-StrictMode -Version 2.0: - Add Test-PropertyExists helper for safe property checks on both hashtables and PSCustomObjects - Guard .SourceInfo access on coverage data hashtables with ContainsKey checks in CoberturaFormatter and CoverageProcessor - Guard .ExecutableLines, .RelativePath, .Procedures, and .ExecutableLineNumbers sub-property access on SourceInfo - Guard .Type access in Get-ProcedureCoverage with fallback - Guard property access in Read-AppJson for missing keys - Fix .Count on null in BCCoverageParser empty file path - Wrap Measure-Object .Sum calls with empty collection check Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The MergeCoverage job runs unconditionally but its steps are already gated by hashFiles checks on cobertura.xml. Adding a comment explaining why job-level gating (via Initialization outputs) is not worth the added complexity for ~20s of savings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use ContainsKey/GetEnumerator for hashtable access. Always write to _merged/ directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds opt-in code coverage collection to AL-Go for GitHub by introducing an AL-based test runner capable of exporting BC coverage data, converting it to Cobertura XML, publishing per-project artifacts, and generating merged/visual summaries in workflows.
Changes:
- Introduces a new
TestRunnermodule to run tests with optional code coverage collection and to convert BC coverage output into Cobertura XML. - Adds new actions (
BuildCodeCoverageSummary,MergeCoverageSummaries) plus workflow/template updates to publish and merge coverage artifacts and render job summaries. - Adds new settings (
enableCodeCoverage,codeCoverageSetup) with schema + documentation updates and supporting Pester tests + test data.
Reviewed changes
Copilot reviewed 61 out of 66 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/CI.yaml | Runs new CodeCoverage Pester test suite in CI matrix. |
| .pre-commit-config.yaml | Excludes intentionally malformed Cobertura XML test fixture from XML lint hook. |
| Actions/.Modules/ReadSettings.psm1 | Adds default values for enableCodeCoverage + codeCoverageSetup. |
| Actions/.Modules/settings.schema.json | Adds schema definitions for new coverage settings. |
| Actions/.Modules/TestRunner/ALTestRunner.psm1 | Public entrypoint to run tests and emit results/coverage. |
| Actions/.Modules/TestRunner/TestResultFormatter.psm1 | Converts test runner results into JUnit/XUnit formats. |
| Actions/.Modules/TestRunner/CoverageProcessor/BCCoverageParser.psm1 | Parses BC coverage exports (CSV/XML) into objects. |
| Actions/.Modules/TestRunner/CoverageProcessor/CoberturaFormatter.psm1 | Formats processed coverage into Cobertura XML. |
| Actions/.Modules/TestRunner/CoverageProcessor/ALSourceParser.psm1 | Parses AL source to map executable lines/procedures for accurate coverage. |
| Actions/.Modules/TestRunner/CoverageProcessor/CoverageProcessor.psm1 | Orchestrates conversion/merge pipeline for BC coverage → Cobertura + stats. |
| Actions/.Modules/TestRunner/Internal/ALTestRunnerInternal.psm1 | Internal test execution loop + optional coverage collection hooks. |
| Actions/.Modules/TestRunner/Internal/ClientContext.ps1 | Implements BC client session plumbing (incl. PS7 SSL handler support). |
| Actions/.Modules/TestRunner/Internal/ClientSessionManager.psm1 | Manages client sessions + SSL verification toggling behavior. |
| Actions/.Modules/TestRunner/Internal/Constants.ps1 | Defines shared constants (test runner IDs, coverage exporter IDs, etc.). |
| Actions/.Modules/TestRunner/Internal/CoverageCollector.psm1 | Collects coverage output/map from the test tool page. |
| Actions/.Modules/TestRunner/Internal/ModuleInit.ps1 | Loads client DLLs and (on PS7) downloads required WCF dependencies. |
| Actions/.Modules/TestRunner/Internal/TestFormHelpers.psm1 | Encapsulates test tool page interactions (filters, isolation, coverage controls). |
| Actions/.Modules/TestRunner/Internal/AadTokenProvider.ps1 | Adds an AAD token provider helper (currently with install-time side effects). |
| Actions/.Modules/TestRunner/Internal/BCPTTestRunnerInternal.psm1 | Adds internal BCPT test runner support (includes AAD helper wiring). |
| Actions/BuildCodeCoverageSummary/action.yaml | New composite action to render coverage summary from Cobertura artifact. |
| Actions/BuildCodeCoverageSummary/BuildCodeCoverageSummary.ps1 | Generates GitHub step summary from {project}/.buildartifacts/CodeCoverage/cobertura.xml. |
| Actions/BuildCodeCoverageSummary/README.md | Documents usage/inputs for coverage summary action. |
| Actions/MergeCoverageSummaries/action.yaml | New composite action to merge multiple Cobertura artifacts. |
| Actions/MergeCoverageSummaries/MergeCoverageSummaries.ps1 | Finds cobertura inputs, merges XML + stats, writes consolidated summary. |
| Actions/MergeCoverageSummaries/CoberturaMerger.psm1 | Implements Cobertura XML line-level merging + stats merging. |
| Actions/MergeCoverageSummaries/README.md | Documents merge action usage, behavior, and expected artifact layout. |
| Actions/CalculateArtifactNames/action.yaml | Adds CodeCoverage artifact name output wiring. |
| Actions/CalculateArtifactNames/CalculateArtifactNames.ps1 | Produces CodeCoverageArtifactsName alongside existing artifact name outputs. |
| Actions/CheckForUpdates/CheckForUpdates.HelperFunctions.ps1 | Ensures PostProcess.needs includes MergeCoverage when present in workflow YAML. |
| Actions/RunPipeline/action.yaml | Adds projectDependenciesJson input and passes through to RunPipeline.ps1. |
| Actions/RunPipeline/RunPipeline.ps1 | Imports test runner, injects coverage-enabled RunTestsInBcContainer override, and converts .dat coverage to Cobertura after tests. |
| RELEASENOTES.md | Notes new (Preview) code coverage feature and points to documentation. |
| Scenarios/settings.md | Documents enableCodeCoverage + codeCoverageSetup and clarifies override interaction. |
| Scenarios/CodeCoverage.md | New scenario doc explaining enabling, config, outputs, limitations, and custom override usage. |
| Templates/AppSource App/.github/workflows/_BuildALGoProject.yaml | Uploads CodeCoverage artifacts and runs BuildCodeCoverageSummary when Cobertura exists. |
| Templates/AppSource App/.github/workflows/PullRequestHandler.yaml | Adds MergeCoverage job and wires StatusCheck needs. |
| Templates/AppSource App/.github/workflows/CICD.yaml | Adds MergeCoverage job and includes it in PostProcess needs. |
| Templates/Per Tenant Extension/.github/workflows/_BuildALGoProject.yaml | Uploads CodeCoverage artifacts and runs BuildCodeCoverageSummary when Cobertura exists. |
| Templates/Per Tenant Extension/.github/workflows/PullRequestHandler.yaml | Adds MergeCoverage job and wires StatusCheck needs. |
| Templates/Per Tenant Extension/.github/workflows/CICD.yaml | Adds MergeCoverage job and includes it in PostProcess needs. |
| Tests/CodeCoverage/ALSourceParser.Test.ps1 | Pester tests for AL source parsing used by coverage formatting. |
| Tests/CodeCoverage/BCCoverageParser.Test.ps1 | Pester tests for parsing BC coverage CSV/XML formats. |
| Tests/CodeCoverage/BuildCodeCoverageSummary.Test.ps1 | Pester tests for summary generation sizing/output behaviors. |
| Tests/CodeCoverage/CoberturaFormatter.Test.ps1 | Pester tests for Cobertura XML generation and stats correctness. |
| Tests/CodeCoverage/CoberturaMerger.Test.ps1 | Pester tests for Cobertura merge behavior and stats recalculation. |
| Tests/CodeCoverage/CoverageProcessor.Test.ps1 | Pester tests for end-to-end conversion/merge to Cobertura + stats JSON. |
| Tests/CodeCoverage/CoverageReportGenerator.Test.ps1 | Pester tests for markdown report generation from Cobertura. |
| Tests/CodeCoverage/MergeCoverageSummaries.Test.ps1 | Pester tests for artifact discovery, merging, and summary constraints. |
| Tests/CodeCoverage/TestData/ALFiles/complex-codeunit.al | Fixture for AL parsing and executable line detection. |
| Tests/CodeCoverage/TestData/ALFiles/sample-codeunit.al | Fixture for AL parsing and procedure mapping. |
| Tests/CodeCoverage/TestData/ALFiles/sample-page.al | Fixture for AL object parsing beyond codeunits. |
| Tests/CodeCoverage/TestData/CoberturaFiles/cobertura-empty.xml | Fixture for empty Cobertura handling. |
| Tests/CodeCoverage/TestData/CoberturaFiles/cobertura-malformed.xml | Fixture for malformed Cobertura handling. |
| Tests/CodeCoverage/TestData/CoberturaFiles/cobertura1.xml | Fixture for Cobertura merge/summary tests. |
| Tests/CodeCoverage/TestData/CoberturaFiles/cobertura2.xml | Fixture for Cobertura merge/summary tests. |
| Tests/CodeCoverage/TestData/CoverageFiles/empty-coverage.dat | Fixture for empty BC coverage input handling. |
| Tests/CodeCoverage/TestData/CoverageFiles/malformed-coverage.dat | Fixture for malformed BC coverage CSV input handling. |
| Tests/CodeCoverage/TestData/CoverageFiles/sample-coverage.dat | Fixture for BC CSV coverage parsing and stats. |
| Tests/CodeCoverage/TestData/CoverageFiles/sample-coverage.xml | Fixture for BC XML coverage parsing and stats. |
| Param( | ||
| [Parameter(HelpMessage = "Path containing downloaded coverage artifacts", Mandatory = $true)] | ||
| [string] $coveragePath, | ||
| [Parameter(HelpMessage = "Path to source code checkout", Mandatory = $false)] | ||
| [string] $sourcePath = '' | ||
| ) | ||
|
|
||
| . (Join-Path -Path $PSScriptRoot -ChildPath "..\AL-Go-Helper.ps1" -Resolve) | ||
| . (Join-Path -Path $PSScriptRoot -ChildPath "..\BuildCodeCoverageSummary\CoverageReportGenerator.ps1" -Resolve) | ||
| Import-Module (Join-Path -Path $PSScriptRoot -ChildPath "CoberturaMerger.psm1" -Resolve) -Force -DisableNameChecking |
There was a problem hiding this comment.
The sourcePath parameter is accepted (and is also an action input), but it is never used in the script. This makes the action API misleading and may confuse users who expect source-path-based normalization. Either remove the parameter/input or use it (e.g., to rewrite/normalize filenames in the merged Cobertura output).
| runs: | ||
| using: composite | ||
| steps: | ||
| - name: run | ||
| shell: ${{ inputs.shell }} | ||
| id: merge | ||
| env: | ||
| _coveragePath: ${{ inputs.coveragePath }} | ||
| _sourcePath: ${{ inputs.sourcePath }} | ||
| run: | | ||
| ${{ github.action_path }}/../Invoke-AlGoAction.ps1 -ActionName "MergeCoverageSummaries" -Action { | ||
| ${{ github.action_path }}/MergeCoverageSummaries.ps1 -coveragePath $ENV:_coveragePath -sourcePath $ENV:_sourcePath | ||
| } |
There was a problem hiding this comment.
action.yaml sets a step id: merge and the README documents an output (mergedCoverageFile), but the composite action doesn't declare any outputs: and the script doesn't write to $GITHUB_OUTPUT. If consumers are expected to read the merged file path, add an action output and write it in MergeCoverageSummaries.ps1; otherwise, update the README to remove the output section.
| function Setup-Enviroment | ||
| ( | ||
| [ValidateSet("PROD","OnPrem")] | ||
| [string] $Environment = $script:DefaultEnvironment, | ||
| [string] $SandboxName = $script:DefaultSandboxName, | ||
| [pscredential] $Credential, | ||
| [pscredential] $Token, | ||
| [string] $ClientId, | ||
| [string] $RedirectUri, | ||
| [string] $AadTenantId | ||
| ) |
There was a problem hiding this comment.
Function name Setup-Enviroment is misspelled (should be Setup-Environment). This reduces discoverability and is easy to propagate into public APIs if exported later. Consider renaming the function and updating internal call sites accordingly.
| foreach($failedTest in $failedTests) | ||
| { | ||
| $test = @{ | ||
| codeunitID = $failedTest.codeunitID; | ||
| codeunitName = $failedTest.name; |
There was a problem hiding this comment.
Write-DisabledTestsJson iterates over $failedTests, which is not defined in this scope (the parameter is $FailedTests). This will result in no disabled tests being written (or a runtime error under strict mode) when the function is used. Update the loop to iterate the parameter and ensure the property names match the objects returned by Get-FailedTestsFromXMLFiles (e.g., codeunitName vs name).
| foreach($failedTest in $failedTests) | |
| { | |
| $test = @{ | |
| codeunitID = $failedTest.codeunitID; | |
| codeunitName = $failedTest.name; | |
| foreach($failedTest in $FailedTests) | |
| { | |
| $test = @{ | |
| codeunitID = $failedTest.codeunitID; | |
| codeunitName = $failedTest.codeunitName; |
| ## Outputs | ||
|
|
||
| | Name | Description | | ||
| |------|-------------| | ||
| | `mergedCoverageFile` | Path to the merged Cobertura XML file | | ||
|
|
There was a problem hiding this comment.
This README documents an output mergedCoverageFile, but the composite action currently doesn't declare any outputs and the script doesn't write to $GITHUB_OUTPUT. Either implement the documented output or adjust this section to match actual behavior.
| BeforeAll { | ||
| . (Join-Path $PSScriptRoot "..\..\Actions\AL-Go-Helper.ps1" -Resolve) | ||
| Import-Module (Join-Path $PSScriptRoot "..\..\Actions\.Modules\TestRunner\CoverageProcessor\BCCoverageParser.psm1" -Resolve) -Force | ||
|
|
||
| $script:testDataPath = Join-Path $PSScriptRoot "TestData\CoverageFiles" | ||
| } |
There was a problem hiding this comment.
These tests build paths using Windows-style backslashes inside Join-Path (e.g., "..\..\Actions\...", "TestData\CoverageFiles"). On Linux runners (CI runs ubuntu-latest with pwsh), backslashes are treated as literal characters, so the paths won't resolve and the tests will fail. Use Join-Path with multiple segments (or forward slashes) to keep the paths platform-independent.
| # Specify the name of the module you want to check/install | ||
| $moduleName = "MSAL.PS" | ||
|
|
||
| # Check if the module is already installed | ||
| if (-not (Get-Module -Name $moduleName -ListAvailable)) { | ||
| # Module is not installed, so install it | ||
| Write-Host "Installing $moduleName..." | ||
| Install-Module -Name $moduleName -Force -Scope AllUsers # Use -Scope CurrentUser or -Scope AllUsers as needed | ||
| } | ||
|
|
||
| Import-Module $moduleName | ||
|
|
There was a problem hiding this comment.
This script installs and imports MSAL.PS as a side effect of dot-sourcing the file. That makes module import non-deterministic (network dependency), can fail in restricted environments, and may prompt/require admin rights. Prefer making MSAL an explicit prerequisite (fail with a clear message when missing) or move installation logic behind an explicit opt-in function rather than running at import time.
| BeforeAll { | ||
| . (Join-Path $PSScriptRoot "..\..\Actions\AL-Go-Helper.ps1" -Resolve) | ||
| Import-Module (Join-Path $PSScriptRoot "..\..\Actions\.Modules\TestRunner\CoverageProcessor\ALSourceParser.psm1" -Resolve) -Force | ||
|
|
||
| $script:testDataPath = Join-Path $PSScriptRoot "TestData\ALFiles" | ||
| } |
There was a problem hiding this comment.
This test uses Windows-style backslashes inside Join-Path (e.g., "..\..\Actions\...", "TestData\ALFiles"). On Linux runners (CI runs ubuntu-latest with pwsh), backslashes are treated as literal characters, so the module/test data paths won't resolve. Use Join-Path with multiple segments (or forward slashes) to make the paths cross-platform.
| foreach($failedTest in $failedTests) | ||
| { | ||
| $methodName = $failedTest.method; | ||
| $errorMessage = $failedTests.message |
There was a problem hiding this comment.
Inside Report-ErrorsInAzureDevOps, $errorMessage is taken from $failedTests.message (the whole collection) instead of the current $failedTest.message. This will produce incorrect/empty log-issue messages or stringify the array unexpectedly. Use the loop variable when emitting each issue.
| $errorMessage = $failedTests.message | |
| $errorMessage = $failedTest.message |
| AutorizationType = 'NavUserPassword' | ||
| TestSuite = if ($parameters.testSuite) { $parameters.testSuite } else { 'DEFAULT' } | ||
| Detailed = $true | ||
| DisableSSLVerification = $true | ||
| ResultsFormat = $resultsFormat | ||
| CodeCoverageTrackingType = $ccTrackingTypeCapture |
There was a problem hiding this comment.
DisableSSLVerification is forced to $true for all coverage-enabled test runs. This disables TLS certificate validation even when the ServiceUrl is HTTPS, which is a security risk and can mask misconfiguration. Consider defaulting to $false and only enabling it when explicitly requested (e.g., via an existing setting/parameter), or infer it based on the ServiceUrl scheme and container configuration.
Code Coverage Support for AL-Go
This adds Code coverage support to AL-Go for GitHub.
Overview
I use a modified version of the internal test runner, somewhat similar to the one in container helper, which support code coverage. On top of that, I have made a custom parser from the raw csv format we get from BC to the industry standard Cobertura XML format. There is also new action to create a visual overview in builds. The raw data, including the Cobertura xml is included in artifacts so the user can further analyze in 3rd party tools.
As AL-Go currently relies on container helper to run tests, this feature currently uses the test override to use the modified test runner. In the future when we move away from container helper, this test runner should completely replace the old one.
As this is a pretty large feature, it is not turned on by default. I also could not test all possible cases, so there might be some situations where tests do not work on the new runner, in which case the old one should be used.
Key Components
Test Runner Module
The
.Modules/TestRunnermodule has been added as a complete package for handling test running and code coverage generation.Core Modules:
BCCoverageParser.psm1- Parses Business Central coverage files (CSV and XML formats from XMLport 130470/130471/130007)ALSourceParser.psm1- Analyzes AL source code to identify objects, procedures, and executable linesCoverageProcessor.psm1- Orchestrates the coverage processing pipelineCoberturaFormatter.psm1- Converts BC coverage data to Cobertura XML formatCoberturaMerger.psm1- Merges multiple Cobertura reportsCoverageReportGenerator.psm1- Generates markdown summariesALTestRunner.psm1- Executes AL test suites with optional coverage collectionActions
RunPipeline- Enhanced to support coverage collection during test executionBuildCodeCoverageSummary- Generates coverage reports and artifactsMergeCoverageSummaries- Combines coverage from multiple projects or test runsSettings
Two new settings control the feature:
{ "enableCodeCoverage": false, "codeCoverageSetup": { "trackingType": "PerRun", "produceCodeCoverageMap": "PerCodeunit", "excludeFilesPattern": ["*.Test.al", *PermissionSet*] } }The feature is opt-in and disabled by default to avoid impact on existing workflows.
Technical Details
Breaking Changes
None. The feature is opt-in. Do note however, that the feature is not currently not supported if test override is already used.
Related to issue: #
✅ Checklist