Skip to content

Commit d6ed965

Browse files
CopilotNateD-MSFT
andauthored
Fix upload-result-diff and azure-login pipeline failures (#197) (#218)
* Fix upload-result-diff and azure-login pipeline failures - Skip Azure Login, Download, Upload, and comparison steps on pull_request events where OIDC secrets are unavailable - Separate the "Upload result diff" step from the "Fail if diff detected" step so uploads succeed independently and failures have clear error messages - Apply fixes to both test-query-health and test-codeql-latest-vs-current jobs Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/9c85d93e-42c1-4451-81ad-2a71ece7bc2e Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Fix empty-string arg passed to test script on PR runs Building the python command with an empty $compareFlag variable caused PowerShell to pass an empty string as a positional arg, producing: "unrecognized arguments: ". Switched to building an argument array and splatting it so the flag is only included when present. Applied to both test-query-health and test-codeql-latest-vs-current. Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/a5ce6b58-e7aa-4b94-b731-d0242b47b40e Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Surface msbuild errors from codeql database create and fail loudly on test failures Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/1a592ad2-4448-490f-ae16-a9c34d72bf79 Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Remove stray .pyc accidentally committed in previous commit Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/1a592ad2-4448-490f-ae16-a9c34d72bf79 Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Optimize pipeline for faster PR runs Removed unnecessary build steps for CA ported queries (covered by the build-all step) and adjusted dependencies in workflow to speed up PR runs (test steps will build as part of work.) Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Add Directory.Build.props to wire WDK/SDK NuGet packages into driver test projects Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/7795692b-bc9c-4c2e-a3d3-18f00aed9bcb Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Fix stampinf, ApiValidator, and Dvl.exe failures in CI tests - Remove <Inf> items from KMDFTestTemplate.vcxproj and CppKMDFTestTemplate.vcxproj so the StampInf MSBuild task is not triggered (stampinf.exe is not available in a NuGet-only WDK environment). These templates exist solely for CodeQL analysis, not for building a production driver, so INF stamping is not needed. - Add <ApiValidatorEnabled>false</ApiValidatorEnabled> to Directory.Build.props. ApiValidator.exe is not included in the WDK NuGet packages and is not available in the CI environment. Disabling it allows the ApplicationForDriversTestTemplate (WindowsApplicationForDrivers10.0 toolset) to compile successfully for CodeQL. - Update dvl_tests.ps1 to locate Dvl.exe dynamically from the NuGet packages directory instead of assuming the traditional system WDK install path C:\Program Files (x86)\Windows Kits\10\Tools\dvl\Dvl.exe. Falls back to the system path for developer machines with a full WDK install, and gracefully skips the dvl command-type tests when Dvl.exe is not found in either location. - Fix typo -test_emtpy -> -test_empty in the second Test-DVL "dvl" call inside Test-Driver (the typo was previously unreachable because the test exited earlier due to the Dvl.exe failure). Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/8484509b-2924-427f-bd9a-63e365e4b404 Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * plan: fix ApiValidator and parallelize test runner Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/71e6036e-2c98-4bb7-85ab-2270cbce9c68 Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Fix ApiValidator failures, harden Dvl.exe check, parallelize test runner - Add src/drivers/test/Directory.Build.targets that overrides the WDK's ApiValidator MSBuild target with an empty target. The WDK NuGet packages reference ApiValidator.exe in WindowsDriver.common.targets but do not ship the binary itself, so the post-build step always failed with MSB3721 for builds using the Universal driver target platform. Replacing the target with a no-op cleanly suppresses the step; API validation is irrelevant for the CodeQL static analysis these tests perform. This is the cause of the 6 still-failing tests (UnsafeCallInGlobalInit, MultithreadedAVCondition, StaticInitializer, DeviceInitApi, FloatSafeExit, FloatUnsafeExit). - Remove the previous <ApiValidatorEnabled>false</ApiValidatorEnabled> workaround from Directory.Build.props -- the WDK targets do not honor that property name, so it had no effect. - dvl_tests.ps1: when Dvl.exe cannot be located in either the NuGet packages directory or the system WDK install path, exit with a clear failure instead of silently skipping the dvl command-type tests. Skipping let regressions go undetected. - build_create_analyze_test.py: parallelize the test runner. Each ql_test uses isolated working/<name>, TestDB/<name>, and AnalysisFiles/<name>.sarif paths, so multiple tests are safe to execute concurrently. Use multiprocessing.pool.ThreadPool (already imported) with a worker count controlled by a new -j/--jobs flag, defaulting to os.cpu_count(). Pass --jobs 1 to fall back to the legacy sequential behaviour. Print output is already protected by print_mutex; added results_mutex around the shared health_df / detailed_health_df DataFrame writes. Refactored the body of the per-test loop into _run_single_test for use by the pool's imap_unordered. Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/71e6036e-2c98-4bb7-85ab-2270cbce9c68 Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Parallelize test-query-health and test-codeql-latest-vs-current jobs Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/694e9d3d-f942-4564-8e96-8af21abeb87a Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Fix LNK1318 mspdbsrv race; wire --jobs into workflow Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/694e9d3d-f942-4564-8e96-8af21abeb87a Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> * Pipeline tweaks: progress log, parallel query compile, scoped pack-version gate, broader publish needs Agent-Logs-Url: https://github.com/microsoft/Windows-Driver-Developer-Supplemental-Tools/sessions/96f4dbad-dab0-45e8-9a14-94efd2c96d27 Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> --------- Signed-off-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: NateD-MSFT <34494373+NateD-MSFT@users.noreply.github.com>
1 parent 7b7ed40 commit d6ed965

7 files changed

Lines changed: 313 additions & 52 deletions

File tree

.github/workflows/build-codeql.yaml

Lines changed: 64 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -52,23 +52,18 @@ jobs:
5252

5353
- name: Build must-fix driver suite
5454
shell: cmd
55-
run: .\codeql-cli\codeql.cmd query compile --check-only mustfix.qls
55+
run: .\codeql-cli\codeql.cmd query compile --check-only --threads=0 mustfix.qls
5656

5757
- name: Build recommended driver suite
5858
shell: cmd
59-
run: .\codeql-cli\codeql.cmd query compile --check-only recommended.qls
60-
61-
- name: Build CA ported queries
62-
shell: cmd
63-
run: .\codeql-cli\codeql.cmd query compile --check-only ported_driver_ca_checks.qls
59+
run: .\codeql-cli\codeql.cmd query compile --check-only --threads=0 recommended.qls
6460

6561
- name: Build all Windows queries
6662
shell: cmd
67-
run: .\codeql-cli\codeql.cmd query compile --check-only .\src
63+
run: .\codeql-cli\codeql.cmd query compile --check-only --threads=0 .\src
6864

6965
test-query-health:
7066
runs-on: windows-latest
71-
needs: build
7267
permissions:
7368
contents: read
7469
packages: write
@@ -111,13 +106,15 @@ jobs:
111106
- name: Add msbuild to PATH
112107
uses: microsoft/setup-msbuild@v2
113108
- name: Azure Login
109+
if: github.event_name != 'pull_request'
114110
uses: azure/login@v2
115111
with:
116112
client-id: ${{ secrets.AZURE_CLIENT_ID }}
117113
tenant-id: ${{ secrets.AZURE_TENANT_ID }}
118114
subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
119115
enable-AzPSSession: true
120116
- name: Download previous results
117+
if: github.event_name != 'pull_request'
121118
uses: azure/powershell@v2
122119
with:
123120
azPSVersion: latest
@@ -127,24 +124,40 @@ jobs:
127124
Get-AzStorageFileContent -ShareName "$env:SHARE_NAME" -Path "detailedfunctiontestresults.xlsx" -Destination $destination -Context $context
128125
- name: Run test script
129126
shell: pwsh
130-
run: python src\drivers\test\build_create_analyze_test.py --codeql_path .\codeql-cli\codeql.exe --no_build --compare_results -v
127+
run: |
128+
# Run per-test build/analyze in parallel inside the script. Default is
129+
# one worker per logical CPU (--jobs <N>); each worker is isolated to
130+
# its own working/, TestDB/, and AnalysisFiles/<name>.sarif paths.
131+
$pyArgs = @('src\drivers\test\build_create_analyze_test.py', '--codeql_path', '.\codeql-cli\codeql.exe', '--no_build', '-v', '--jobs', "$env:NUMBER_OF_PROCESSORS")
132+
if ("${{ github.event_name }}" -ne "pull_request") {
133+
$pyArgs += '--compare_results'
134+
}
135+
python @pyArgs
131136
- name: Upload result diff
132-
if: ${{ hashFiles('diffdetailedfunctiontestresults.xlsx') != '' }} # Only upload if there are changes
137+
if: github.event_name != 'pull_request' && hashFiles('diffdetailedfunctiontestresults.xlsx') != ''
133138
uses: azure/powershell@v2
134139
with:
135140
azPSVersion: latest
136141
inlineScript: |
137142
Update-AzConfig -DisplayBreakingChangeWarning $false
138143
$context = New-AzStorageContext -StorageAccountName "$env:ACCOUNT_NAME" -UseConnectedAccount -EnableFileBackupRequestIntent
139144
Set-AzStorageFileContent -ShareName "$env:SHARE_NAME" -Source "diffdetailedfunctiontestresults.xlsx" -Path "health-diffdetailedfunctiontestresults.xlsx" -Context $context -Force
140-
exit 1
145+
- name: Fail if result diff detected
146+
if: github.event_name != 'pull_request' && hashFiles('diffdetailedfunctiontestresults.xlsx') != ''
147+
shell: pwsh
148+
run: |
149+
Write-Host "::error::Test results differ from the stored baseline. The diff has been uploaded to Azure Storage as 'health-diffdetailedfunctiontestresults.xlsx'. Please review."
150+
exit 1
141151
142152

143153
test-codeql-latest-vs-current:
144-
# Tests if the latest codeql version produces the same results as the current version.
154+
# Tests if the latest codeql version produces the same results as the current version.
155+
# Runs in parallel with `test-query-health` (no `needs:` dependency) to halve the
156+
# pipeline's wall-clock time. It is independent: it downloads its own (latest)
157+
# CodeQL CLI and runs the same per-test build/analyze cycle. `continue-on-error`
158+
# below means failures here never block the workflow regardless of order.
145159
runs-on: windows-latest
146160
continue-on-error: true # Allow script to return non-zero exit code
147-
needs: [build,test-query-health]
148161
permissions:
149162
contents: read
150163
packages: write
@@ -153,10 +166,6 @@ jobs:
153166
ACCOUNT_NAME: ${{ secrets.ACCOUNT_NAME }}
154167
SHARE_NAME: ${{ secrets.SHARE_NAME }}
155168
steps:
156-
- name: Check Prev Job
157-
if: ${{ needs.test-query-health.result == 'failure' }}
158-
shell: pwsh
159-
run: exit 1
160169
- name: Enable long git paths
161170
shell: cmd
162171
run: git config --global core.longpaths true
@@ -194,13 +203,15 @@ jobs:
194203
- name: Add msbuild to PATH
195204
uses: microsoft/setup-msbuild@v2
196205
- name: Azure Login
206+
if: github.event_name != 'pull_request'
197207
uses: azure/login@v2
198208
with:
199209
client-id: ${{ secrets.AZURE_CLIENT_ID }}
200210
tenant-id: ${{ secrets.AZURE_TENANT_ID }}
201211
subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
202212
enable-AzPSSession: true
203213
- name: Download previous results
214+
if: github.event_name != 'pull_request'
204215
uses: azure/powershell@v2
205216
with:
206217
azPSVersion: latest
@@ -210,16 +221,29 @@ jobs:
210221
Get-AzStorageFileContent -ShareName "$env:SHARE_NAME" -Path "detailedfunctiontestresults.xlsx" -Destination $destination -Context $context
211222
- name: Run test script
212223
shell: pwsh
213-
run: python src\drivers\test\build_create_analyze_test.py --codeql_path .\codeql-cli\codeql.exe --no_build --compare_results -v
224+
run: |
225+
# Run per-test build/analyze in parallel inside the script. Default is
226+
# one worker per logical CPU (--jobs <N>); each worker is isolated to
227+
# its own working/, TestDB/, and AnalysisFiles/<name>.sarif paths.
228+
$pyArgs = @('src\drivers\test\build_create_analyze_test.py', '--codeql_path', '.\codeql-cli\codeql.exe', '--no_build', '-v', '--jobs', "$env:NUMBER_OF_PROCESSORS")
229+
if ("${{ github.event_name }}" -ne "pull_request") {
230+
$pyArgs += '--compare_results'
231+
}
232+
python @pyArgs
214233
- name: Upload result diff
215-
if: ${{ hashFiles('diffdetailedfunctiontestresults.xlsx') != '' }} # Only upload if there are changes
234+
if: github.event_name != 'pull_request' && hashFiles('diffdetailedfunctiontestresults.xlsx') != ''
216235
uses: azure/powershell@v2
217236
with:
218237
azPSVersion: latest
219238
inlineScript: |
220239
$context = New-AzStorageContext -StorageAccountName "$env:ACCOUNT_NAME" -UseConnectedAccount -EnableFileBackupRequestIntent
221240
Set-AzStorageFileContent -ShareName "$env:SHARE_NAME" -Source "diffdetailedfunctiontestresults.xlsx" -Path "version-diffdetailedfunctiontestresults.xlsx" -Context $context -Force
222-
exit 1
241+
- name: Fail if result diff detected
242+
if: github.event_name != 'pull_request' && hashFiles('diffdetailedfunctiontestresults.xlsx') != ''
243+
shell: pwsh
244+
run: |
245+
Write-Host "::error::Test results from latest CodeQL version differ from the stored baseline. The diff has been uploaded to Azure Storage as 'version-diffdetailedfunctiontestresults.xlsx'. Please review."
246+
exit 1
223247
- name: Save Latest Version
224248
if: ${{ hashFiles('diffdetailedfunctiontestresults.xlsx') == '' }} # Only if there were no differences
225249
uses: actions/upload-artifact@v4
@@ -230,7 +254,13 @@ jobs:
230254
231255
test-pack-version-update:
232256
runs-on: windows-latest
233-
needs: build
257+
# Only enforce qlpack version bumps when the change is actually heading to
258+
# `main`. We routinely stage many commits in `development` and bump the
259+
# qlpack version once when promoting to `main`, so requiring a bump on
260+
# every `development`-targeted PR/push is noise.
261+
if: |
262+
(github.event_name == 'pull_request' && github.base_ref == 'main') ||
263+
(github.event_name != 'pull_request' && github.ref == 'refs/heads/main')
234264
permissions:
235265
contents: read
236266
packages: write
@@ -272,7 +302,6 @@ jobs:
272302
}
273303
test-create-dvl:
274304
runs-on: windows-latest
275-
needs: build
276305
permissions:
277306
contents: read
278307
packages: write
@@ -319,7 +348,19 @@ jobs:
319348
publish:
320349
runs-on: windows-latest
321350
continue-on-error: true
322-
needs: [build, test-pack-version-update, test-query-health]
351+
needs: [build, test-pack-version-update, test-query-health, test-codeql-latest-vs-current, test-create-dvl]
352+
# Run when all required gates pass. `test-pack-version-update` is skipped
353+
# for non-`main` targets (see its `if:` above), so allow `success` *or*
354+
# `skipped`. `test-codeql-latest-vs-current` is `continue-on-error: true`,
355+
# which already produces a `success` result for `needs`, so we don't need
356+
# special handling for it here -- listing it in `needs` just makes publish
357+
# wait for it to finish before running.
358+
if: |
359+
always() &&
360+
needs.build.result == 'success' &&
361+
needs.test-query-health.result == 'success' &&
362+
needs.test-create-dvl.result == 'success' &&
363+
(needs.test-pack-version-update.result == 'success' || needs.test-pack-version-update.result == 'skipped')
323364
permissions:
324365
contents: read
325366
packages: write
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<Project>
2+
<!--
3+
Auto-imported by MSBuild for every project under src\drivers\test\ (both the
4+
TestTemplates\ originals and the generated working\ copies used by
5+
build_create_analyze_test.py). Wires the WDK and Windows SDK CPP NuGet
6+
packages restored by the workflow's `nuget restore ... -PackagesDirectory
7+
.\packages\` step into the driver template projects so they can find wdm.h,
8+
ntifs.h, wdf.h, etc.
9+
10+
Mirrors the pattern used by microsoft/Windows-driver-samples
11+
(see https://github.com/microsoft/Windows-driver-samples/blob/main/Directory.Build.props),
12+
keeping the package set and versions in sync with packages.config in this
13+
same directory.
14+
-->
15+
<PropertyGroup>
16+
<WdkNuGetPackagesDir>$(MSBuildThisFileDirectory)..\..\..\packages\</WdkNuGetPackagesDir>
17+
</PropertyGroup>
18+
<Import Project="$(WdkNuGetPackagesDir)Microsoft.Windows.WDK.x64.10.0.26100.6584\build\native\Microsoft.Windows.WDK.x64.props" Condition="Exists('$(WdkNuGetPackagesDir)Microsoft.Windows.WDK.x64.10.0.26100.6584\build\native\Microsoft.Windows.WDK.x64.props') and '$(Platform)' == 'x64'" />
19+
<Import Project="$(WdkNuGetPackagesDir)Microsoft.Windows.WDK.arm64.10.0.26100.6584\build\native\Microsoft.Windows.WDK.arm64.props" Condition="Exists('$(WdkNuGetPackagesDir)Microsoft.Windows.WDK.arm64.10.0.26100.6584\build\native\Microsoft.Windows.WDK.arm64.props') and '$(Platform)' == 'ARM64'" />
20+
<Import Project="$(WdkNuGetPackagesDir)Microsoft.Windows.SDK.CPP.x64.10.0.26100.6584\build\native\Microsoft.Windows.SDK.cpp.x64.props" Condition="Exists('$(WdkNuGetPackagesDir)Microsoft.Windows.SDK.CPP.x64.10.0.26100.6584\build\native\Microsoft.Windows.SDK.cpp.x64.props') and '$(Platform)' == 'x64'" />
21+
<Import Project="$(WdkNuGetPackagesDir)Microsoft.Windows.SDK.CPP.arm64.10.0.26100.6584\build\native\Microsoft.Windows.SDK.cpp.arm64.props" Condition="Exists('$(WdkNuGetPackagesDir)Microsoft.Windows.SDK.CPP.arm64.10.0.26100.6584\build\native\Microsoft.Windows.SDK.cpp.arm64.props') and '$(Platform)' == 'ARM64'" />
22+
<Import Project="$(WdkNuGetPackagesDir)Microsoft.Windows.SDK.CPP.10.0.26100.6584\build\native\Microsoft.Windows.SDK.cpp.props" Condition="Exists('$(WdkNuGetPackagesDir)Microsoft.Windows.SDK.CPP.10.0.26100.6584\build\native\Microsoft.Windows.SDK.cpp.props')" />
23+
</Project>
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<Project>
2+
<!--
3+
Auto-imported by MSBuild AFTER each project's main targets (including the
4+
WDK targets pulled in by Directory.Build.props), for every project under
5+
src\drivers\test\.
6+
7+
Override the WDK's `ApiValidator` post-build target with an empty target.
8+
The WDK NuGet packages restored by the CI workflow do not ship the
9+
`ApiValidator.exe` binary even though they reference its path in
10+
`WindowsDriver.common.targets`. Without this override the target fails
11+
with:
12+
13+
error MSB3721: The command "...\ApiValidator.exe -DriverPackagePath:..."
14+
exited with code 1
15+
16+
breaking the `codeql database create` invocation for tests built with the
17+
Universal driver target platform (UnsafeCallInGlobalInit,
18+
MultithreadedAVCondition, StaticInitializer, DeviceInitApi, FloatSafeExit,
19+
FloatUnsafeExit, ...).
20+
21+
API validation is irrelevant to the CodeQL static analysis these tests
22+
perform, so replacing the target with a no-op is safe here. Setting
23+
`ApiValidatorExePath` to an empty value would still leave the target
24+
running an empty exec; replacing the target itself is the most reliable
25+
suppression.
26+
-->
27+
<Target Name="ApiValidator" />
28+
</Project>

src/drivers/test/TestTemplates/CppKMDFTestTemplate/CppKMDFTestTemplate.vcxproj

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@
3333
<ClInclude Include="Queue.h" />
3434
<ClInclude Include="Trace.h" />
3535
</ItemGroup>
36-
<ItemGroup>
37-
<Inf Include="CppKMDFTestTemplate.inf" />
38-
</ItemGroup>
3936
<PropertyGroup Label="Globals">
4037
<ProjectGuid>{0B59834A-7319-449C-822B-09B4CFAC9752}</ProjectGuid>
4138
<TemplateGuid>{8c0e3d8b-df43-455b-815a-4a0e72973bc6}</TemplateGuid>

src/drivers/test/TestTemplates/KMDFTestTemplate/KMDFTestTemplate.vcxproj

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,6 @@
3333
<ClInclude Include="Queue.h" />
3434
<ClInclude Include="Trace.h" />
3535
</ItemGroup>
36-
<ItemGroup>
37-
<Inf Include="KMDFTestTemplate.inf" />
38-
</ItemGroup>
3936
<PropertyGroup Label="Globals">
4037
<ProjectGuid>{AD97E1A9-DDBC-4BC2-B3B8-95D11062B471}</ProjectGuid>
4138
<TemplateGuid>{8c0e3d8b-df43-455b-815a-4a0e72973bc6}</TemplateGuid>

0 commit comments

Comments
 (0)