diff --git a/.github/workflows/build-codeql.yaml b/.github/workflows/build-codeql.yaml index e4a1fe4e..79611d2b 100644 --- a/.github/workflows/build-codeql.yaml +++ b/.github/workflows/build-codeql.yaml @@ -52,23 +52,18 @@ jobs: - name: Build must-fix driver suite shell: cmd - run: .\codeql-cli\codeql.cmd query compile --check-only mustfix.qls + run: .\codeql-cli\codeql.cmd query compile --check-only --threads=0 mustfix.qls - name: Build recommended driver suite shell: cmd - run: .\codeql-cli\codeql.cmd query compile --check-only recommended.qls - - - name: Build CA ported queries - shell: cmd - run: .\codeql-cli\codeql.cmd query compile --check-only ported_driver_ca_checks.qls + run: .\codeql-cli\codeql.cmd query compile --check-only --threads=0 recommended.qls - name: Build all Windows queries shell: cmd - run: .\codeql-cli\codeql.cmd query compile --check-only .\src + run: .\codeql-cli\codeql.cmd query compile --check-only --threads=0 .\src test-query-health: runs-on: windows-latest - needs: build permissions: contents: read packages: write @@ -111,6 +106,7 @@ jobs: - name: Add msbuild to PATH uses: microsoft/setup-msbuild@v2 - name: Azure Login + if: github.event_name != 'pull_request' uses: azure/login@v2 with: client-id: ${{ secrets.AZURE_CLIENT_ID }} @@ -118,6 +114,7 @@ jobs: subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} enable-AzPSSession: true - name: Download previous results + if: github.event_name != 'pull_request' uses: azure/powershell@v2 with: azPSVersion: latest @@ -127,9 +124,17 @@ jobs: Get-AzStorageFileContent -ShareName "$env:SHARE_NAME" -Path "detailedfunctiontestresults.xlsx" -Destination $destination -Context $context - name: Run test script shell: pwsh - run: python src\drivers\test\build_create_analyze_test.py --codeql_path .\codeql-cli\codeql.exe --no_build --compare_results -v + run: | + # Run per-test build/analyze in parallel inside the script. Default is + # one worker per logical CPU (--jobs ); each worker is isolated to + # its own working/, TestDB/, and AnalysisFiles/.sarif paths. + $pyArgs = @('src\drivers\test\build_create_analyze_test.py', '--codeql_path', '.\codeql-cli\codeql.exe', '--no_build', '-v', '--jobs', "$env:NUMBER_OF_PROCESSORS") + if ("${{ github.event_name }}" -ne "pull_request") { + $pyArgs += '--compare_results' + } + python @pyArgs - name: Upload result diff - if: ${{ hashFiles('diffdetailedfunctiontestresults.xlsx') != '' }} # Only upload if there are changes + if: github.event_name != 'pull_request' && hashFiles('diffdetailedfunctiontestresults.xlsx') != '' uses: azure/powershell@v2 with: azPSVersion: latest @@ -137,14 +142,22 @@ jobs: Update-AzConfig -DisplayBreakingChangeWarning $false $context = New-AzStorageContext -StorageAccountName "$env:ACCOUNT_NAME" -UseConnectedAccount -EnableFileBackupRequestIntent Set-AzStorageFileContent -ShareName "$env:SHARE_NAME" -Source "diffdetailedfunctiontestresults.xlsx" -Path "health-diffdetailedfunctiontestresults.xlsx" -Context $context -Force - exit 1 + - name: Fail if result diff detected + if: github.event_name != 'pull_request' && hashFiles('diffdetailedfunctiontestresults.xlsx') != '' + shell: pwsh + run: | + Write-Host "::error::Test results differ from the stored baseline. The diff has been uploaded to Azure Storage as 'health-diffdetailedfunctiontestresults.xlsx'. Please review." + exit 1 test-codeql-latest-vs-current: - # Tests if the latest codeql version produces the same results as the current version. + # Tests if the latest codeql version produces the same results as the current version. + # Runs in parallel with `test-query-health` (no `needs:` dependency) to halve the + # pipeline's wall-clock time. It is independent: it downloads its own (latest) + # CodeQL CLI and runs the same per-test build/analyze cycle. `continue-on-error` + # below means failures here never block the workflow regardless of order. runs-on: windows-latest continue-on-error: true # Allow script to return non-zero exit code - needs: [build,test-query-health] permissions: contents: read packages: write @@ -153,10 +166,6 @@ jobs: ACCOUNT_NAME: ${{ secrets.ACCOUNT_NAME }} SHARE_NAME: ${{ secrets.SHARE_NAME }} steps: - - name: Check Prev Job - if: ${{ needs.test-query-health.result == 'failure' }} - shell: pwsh - run: exit 1 - name: Enable long git paths shell: cmd run: git config --global core.longpaths true @@ -194,6 +203,7 @@ jobs: - name: Add msbuild to PATH uses: microsoft/setup-msbuild@v2 - name: Azure Login + if: github.event_name != 'pull_request' uses: azure/login@v2 with: client-id: ${{ secrets.AZURE_CLIENT_ID }} @@ -201,6 +211,7 @@ jobs: subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} enable-AzPSSession: true - name: Download previous results + if: github.event_name != 'pull_request' uses: azure/powershell@v2 with: azPSVersion: latest @@ -210,16 +221,29 @@ jobs: Get-AzStorageFileContent -ShareName "$env:SHARE_NAME" -Path "detailedfunctiontestresults.xlsx" -Destination $destination -Context $context - name: Run test script shell: pwsh - run: python src\drivers\test\build_create_analyze_test.py --codeql_path .\codeql-cli\codeql.exe --no_build --compare_results -v + run: | + # Run per-test build/analyze in parallel inside the script. Default is + # one worker per logical CPU (--jobs ); each worker is isolated to + # its own working/, TestDB/, and AnalysisFiles/.sarif paths. + $pyArgs = @('src\drivers\test\build_create_analyze_test.py', '--codeql_path', '.\codeql-cli\codeql.exe', '--no_build', '-v', '--jobs', "$env:NUMBER_OF_PROCESSORS") + if ("${{ github.event_name }}" -ne "pull_request") { + $pyArgs += '--compare_results' + } + python @pyArgs - name: Upload result diff - if: ${{ hashFiles('diffdetailedfunctiontestresults.xlsx') != '' }} # Only upload if there are changes + if: github.event_name != 'pull_request' && hashFiles('diffdetailedfunctiontestresults.xlsx') != '' uses: azure/powershell@v2 with: azPSVersion: latest inlineScript: | $context = New-AzStorageContext -StorageAccountName "$env:ACCOUNT_NAME" -UseConnectedAccount -EnableFileBackupRequestIntent Set-AzStorageFileContent -ShareName "$env:SHARE_NAME" -Source "diffdetailedfunctiontestresults.xlsx" -Path "version-diffdetailedfunctiontestresults.xlsx" -Context $context -Force - exit 1 + - name: Fail if result diff detected + if: github.event_name != 'pull_request' && hashFiles('diffdetailedfunctiontestresults.xlsx') != '' + shell: pwsh + run: | + 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." + exit 1 - name: Save Latest Version if: ${{ hashFiles('diffdetailedfunctiontestresults.xlsx') == '' }} # Only if there were no differences uses: actions/upload-artifact@v4 @@ -230,7 +254,13 @@ jobs: test-pack-version-update: runs-on: windows-latest - needs: build + # Only enforce qlpack version bumps when the change is actually heading to + # `main`. We routinely stage many commits in `development` and bump the + # qlpack version once when promoting to `main`, so requiring a bump on + # every `development`-targeted PR/push is noise. + if: | + (github.event_name == 'pull_request' && github.base_ref == 'main') || + (github.event_name != 'pull_request' && github.ref == 'refs/heads/main') permissions: contents: read packages: write @@ -272,7 +302,6 @@ jobs: } test-create-dvl: runs-on: windows-latest - needs: build permissions: contents: read packages: write @@ -319,7 +348,19 @@ jobs: publish: runs-on: windows-latest continue-on-error: true - needs: [build, test-pack-version-update, test-query-health] + needs: [build, test-pack-version-update, test-query-health, test-codeql-latest-vs-current, test-create-dvl] + # Run when all required gates pass. `test-pack-version-update` is skipped + # for non-`main` targets (see its `if:` above), so allow `success` *or* + # `skipped`. `test-codeql-latest-vs-current` is `continue-on-error: true`, + # which already produces a `success` result for `needs`, so we don't need + # special handling for it here -- listing it in `needs` just makes publish + # wait for it to finish before running. + if: | + always() && + needs.build.result == 'success' && + needs.test-query-health.result == 'success' && + needs.test-create-dvl.result == 'success' && + (needs.test-pack-version-update.result == 'success' || needs.test-pack-version-update.result == 'skipped') permissions: contents: read packages: write diff --git a/src/drivers/test/Directory.Build.props b/src/drivers/test/Directory.Build.props new file mode 100644 index 00000000..24be29da --- /dev/null +++ b/src/drivers/test/Directory.Build.props @@ -0,0 +1,23 @@ + + + + $(MSBuildThisFileDirectory)..\..\..\packages\ + + + + + + + diff --git a/src/drivers/test/Directory.Build.targets b/src/drivers/test/Directory.Build.targets new file mode 100644 index 00000000..2cab4ad5 --- /dev/null +++ b/src/drivers/test/Directory.Build.targets @@ -0,0 +1,28 @@ + + + + diff --git a/src/drivers/test/TestTemplates/CppKMDFTestTemplate/CppKMDFTestTemplate.vcxproj b/src/drivers/test/TestTemplates/CppKMDFTestTemplate/CppKMDFTestTemplate.vcxproj index b5e8b51c..3a5309ad 100644 --- a/src/drivers/test/TestTemplates/CppKMDFTestTemplate/CppKMDFTestTemplate.vcxproj +++ b/src/drivers/test/TestTemplates/CppKMDFTestTemplate/CppKMDFTestTemplate.vcxproj @@ -33,9 +33,6 @@ - - - {0B59834A-7319-449C-822B-09B4CFAC9752} {8c0e3d8b-df43-455b-815a-4a0e72973bc6} diff --git a/src/drivers/test/TestTemplates/KMDFTestTemplate/KMDFTestTemplate.vcxproj b/src/drivers/test/TestTemplates/KMDFTestTemplate/KMDFTestTemplate.vcxproj index eee26610..7898a93e 100644 --- a/src/drivers/test/TestTemplates/KMDFTestTemplate/KMDFTestTemplate.vcxproj +++ b/src/drivers/test/TestTemplates/KMDFTestTemplate/KMDFTestTemplate.vcxproj @@ -33,9 +33,6 @@ - - - {AD97E1A9-DDBC-4BC2-B3B8-95D11062B471} {8c0e3d8b-df43-455b-815a-4a0e72973bc6} diff --git a/src/drivers/test/build_create_analyze_test.py b/src/drivers/test/build_create_analyze_test.py index 2a3115f1..3be48abc 100644 --- a/src/drivers/test/build_create_analyze_test.py +++ b/src/drivers/test/build_create_analyze_test.py @@ -22,9 +22,64 @@ print_mutex = threading.Lock() +results_mutex = threading.Lock() health_df = pd.DataFrame() detailed_health_df = pd.DataFrame() +# Progress reporting for run_tests(). ``_progress_total`` is the total number +# of tests being executed in the current run; ``_progress_started`` is the +# number of tests for which a worker has begun execution. Both are guarded by +# ``_progress_mutex`` so the "[N/total]" prefix is consistent across parallel +# workers. +_progress_mutex = threading.Lock() +_progress_started = 0 +_progress_total = 0 + + +def _next_progress_label(): + """Return a "[N/total]" string and atomically advance the counter.""" + global _progress_started + with _progress_mutex: + _progress_started += 1 + return "[{}/{}]".format(_progress_started, _progress_total) + +# Names of tests for which `codeql database create` failed. Tracked here +# (rather than only in `run_tests`) so failures are still surfaced when the +# script is invoked with --build_database_only, which legitimately returns +# None from `run_test` on success. +db_create_failures = [] + + +def _build_subprocess_env(): + """ + Build an environment dict for subprocess invocations of msbuild/link.exe so + that each concurrent invocation talks to its own private ``mspdbsrv.exe`` + PDB server. + + When the parallel test runner spawns multiple `codeql database create` -> + `msbuild` -> `link.exe` chains concurrently, all link.exe processes + inherit the same per-user `_MSPDBSRV_ENDPOINT_` and contend on a single + `mspdbsrv.exe` for PDB RPC. Under load this races and the RPC server + occasionally becomes unavailable, producing: + + LINK : fatal error LNK1318: Unexpected PDB error; RPC (23) '(0x000006BA)' + + Setting a unique `_MSPDBSRV_ENDPOINT_` per invocation makes link.exe + spawn its own dedicated mspdbsrv with a unique RPC endpoint, eliminating + cross-worker contention. This is the Microsoft-recommended fix for parallel + MSVC builds. See https://learn.microsoft.com/cpp/error-messages/tool-errors/linker-tools-error-lnk1318. + """ + env = os.environ.copy() + env["_MSPDBSRV_ENDPOINT_"] = "wddst_{pid}_{tid}_{n}".format( + pid=os.getpid(), + tid=threading.get_ident(), + n=next(_endpoint_counter), + ) + return env + + +_endpoint_counter = itertools.count() + def print_conditionally(*message): """ @@ -363,13 +418,18 @@ def db_create_for_external_driver(sln_file, config, platform): out2 = subprocess.run([codeql_path, "database", "create", db_loc, "--overwrite", "-l", "cpp", "--source-root="+workdir, "--command=msbuild "+ sln_file+ " -clp:Verbosity=m -t:clean,build -property:Configuration="+config+" -property:Platform="+platform + " -p:TargetVersion=Windows10 -p:SignToolWS=/fdws -p:DriverCFlagAddOn=/wd4996 -noLogo" ], cwd=workdir, - shell=True, capture_output=no_output ) + shell=True, capture_output=True, env=_build_subprocess_env() ) if out2.returncode != 0: print("Error in codeql database create: " + db_loc) + print("Return code: " + str(out2.returncode)) try: - print(out2.stderr.decode()) - except: - print(out2.stderr) + print("STDOUT:\n" + out2.stdout.decode(errors="replace")) + except Exception: + print("STDOUT:", out2.stdout) + try: + print("STDERR:\n" + out2.stderr.decode(errors="replace")) + except Exception: + print("STDERR:", out2.stderr) return None else: @@ -405,16 +465,34 @@ def create_codeql_test_database(ql_test): print_conditionally(" - Database location: " + db_loc) print_conditionally(" - Source directory: " + source_dir) print_conditionally(" - Command to run: " + str(codeql_command)) + # Always capture output so that on failure we can surface the underlying + # msbuild / tracer diagnostics (which CodeQL writes mostly to stdout). + # ``env=`` injects a unique ``_MSPDBSRV_ENDPOINT_`` so concurrent workers + # do not race on a shared mspdbsrv.exe (LNK1318 RPC errors). out2 = subprocess.run(codeql_command, - shell=True, capture_output=no_output ) + shell=True, capture_output=True, env=_build_subprocess_env() ) if out2.returncode != 0: print("Error in codeql database create: " + ql_test.get_ql_name()) + print("Command: " + str(codeql_command)) + print("Return code: " + str(out2.returncode)) try: - print("ERROR MESSAGE:", out2.stderr.decode() ) - except: - print("ERROR MESSAGE:", out2.stderr) + print("STDOUT:\n" + out2.stdout.decode(errors="replace")) + except Exception: + print("STDOUT:", out2.stdout) + try: + print("STDERR:\n" + out2.stderr.decode(errors="replace")) + except Exception: + print("STDERR:", out2.stderr) + db_create_failures.append(ql_test.get_ql_name()) return None + elif not no_output: + # In more_verbose mode, still echo the captured output for parity + # with previous behaviour. + try: + print(out2.stdout.decode(errors="replace")) + except Exception: + pass return db_loc @@ -537,7 +615,7 @@ def run_test(ql_test): # Print test attributes print_mutex.acquire() - print("\nRunning test: " + ql_test.get_ql_name()) + print("\n" + _next_progress_label() + " Running test: " + ql_test.get_ql_name()) print_conditionally(" - Template: ", ql_test.get_template(), "\n", "- Driver Framework: ", ql_test.get_ql_type(), "\n", @@ -803,6 +881,36 @@ def compare_health_results(curr_results_path): os.remove(prev_results) exit(0) +def _run_single_test(ql_test): + """ + Run a single ql_test and update the shared results DataFrames. + Returns the test name on failure, or None on success. Designed to be + safe to invoke concurrently from a thread pool: each ql_test uses its + own working/, TestDB/, and AnalysisFiles/.sarif paths, and shared + DataFrame mutations are guarded by ``results_mutex``. + """ + result_sarif = run_test(ql_test) + if args.build_database_only: + return None + if not result_sarif: + with print_mutex: + print("Error running test: " + ql_test.get_ql_name(), "Skipping...") + return ql_test.get_ql_name() + try: + analysis_results, detailed_analysis_results = sarif_results(ql_test, result_sarif) + except Exception as e: + with print_mutex: + print("Error reading sarif results for " + ql_test.get_ql_name() + ": " + str(e)) + return ql_test.get_ql_name() + with results_mutex: + health_df.at[ql_test.get_ql_name(), "Result"] = str( + int(analysis_results['error']) + + int(analysis_results['warning']) + + int(analysis_results['note'])) + detailed_health_df.at[ql_test.get_ql_name(), "Result"] = str(detailed_analysis_results) + return None + + def run_tests(ql_tests_dict): """ Run the given CodeQL tests. @@ -814,16 +922,40 @@ def run_tests(ql_tests_dict): None """ ql_tests_with_attributes = parse_attributes(ql_tests_dict) - - for ql_test in ql_tests_with_attributes: - result_sarif = run_test(ql_test) - if not args.build_database_only: - if not result_sarif: - print("Error running test: " + ql_test.get_ql_name(),"Skipping...") - continue - analysis_results, detailed_analysis_results = sarif_results(ql_test, result_sarif) - health_df.at[ql_test.get_ql_name(), "Result"] = str(int(analysis_results['error'])+int(analysis_results['warning'])+int(analysis_results['note'])) - detailed_health_df.at[ql_test.get_ql_name(), "Result"] = str(detailed_analysis_results) + + total_tests = len(ql_tests_with_attributes) + failed_tests = [] + + # Reset the shared "[N/total]" progress counter for this run so each + # invocation of run_tests starts at [1/total]. + global _progress_started, _progress_total + with _progress_mutex: + _progress_started = 0 + _progress_total = total_tests + + # Each ql_test runs against its own isolated working/, TestDB/, and + # AnalysisFiles/.sarif paths, so it is safe to execute multiple + # tests concurrently. ``--jobs`` controls the worker count; the default + # of ``os.cpu_count()`` mirrors what `msbuild` and `codeql` do internally + # for their own parallelism. Use jobs=1 to fall back to the legacy + # sequential behaviour (useful when debugging a single test). + jobs = max(1, args.jobs) if args.jobs is not None else (os.cpu_count() or 1) + if jobs == 1: + for ql_test in ql_tests_with_attributes: + failed = _run_single_test(ql_test) + if failed is not None: + failed_tests.append(failed) + else: + with print_mutex: + print("Running " + str(total_tests) + " tests with " + str(jobs) + " parallel workers") + pool = ThreadPool(jobs) + try: + for failed in pool.imap_unordered(_run_single_test, ql_tests_with_attributes): + if failed is not None: + failed_tests.append(failed) + finally: + pool.close() + pool.join() # save results result_file = "functiontestresults.xlsx" @@ -839,7 +971,31 @@ def run_tests(ql_tests_dict): local_system_info_df.to_excel(writer, sheet_name="Local System Info") if args.compare_results: compare_health_results("detailed"+result_file) - + + # Surface failures so the CI job no longer reports green when nothing + # actually ran. Database-creation failures are tracked globally so they + # are reported even in --build_database_only mode where `run_test` + # legitimately returns None on success. + if not args.build_database_only and failed_tests: + print("\n==== Test summary ====") + print("Total tests: " + str(total_tests)) + print("Failed tests: " + str(len(failed_tests))) + for name in failed_tests: + print(" - " + name) + if len(failed_tests) == total_tests: + print("ERROR: All tests failed or were skipped. Failing the job.") + else: + print("ERROR: One or more tests failed or were skipped. Failing the job.") + sys.exit(1) + + if db_create_failures: + print("\n==== Database creation summary ====") + print("Failed database creations: " + str(len(db_create_failures))) + for name in db_create_failures: + print(" - " + name) + print("ERROR: One or more `codeql database create` invocations failed. Failing the job.") + sys.exit(1) + def find_g_template_dir(template): """ Finds the directory of the given template. @@ -894,6 +1050,7 @@ def find_sln_file(path): parser.add_argument('--compare_results_no_build',help='Compare results to previous run',type=str,required=False,) parser.add_argument('--codeql_path', help='Path to the codeql executable',type=str,required=False,) parser.add_argument('--build_database_only', help='Build database only',action='store_true',required=False,) + parser.add_argument('-j', '--jobs', help='Number of tests to run in parallel (default: number of CPU cores). Use 1 to disable parallelism.', type=int, required=False) args = parser.parse_args() if args.codeql_path: diff --git a/src/drivers/test/dvl_tests/dvl_tests.ps1 b/src/drivers/test/dvl_tests/dvl_tests.ps1 index 6982b7dd..f9fbbd2a 100644 --- a/src/drivers/test/dvl_tests/dvl_tests.ps1 +++ b/src/drivers/test/dvl_tests/dvl_tests.ps1 @@ -18,6 +18,25 @@ param( $starting_location = Get-Location $platforms = @("x64", "arm64") $configurations = @("Debug", "Release") + +# Locate Dvl.exe: prefer the copy shipped inside the WDK NuGet package so that +# the test works in CI environments that have the NuGet packages restored but no +# system-wide WDK installation. Fall back to the traditional WDK install path +# for developer machines that have the full WDK installed. If Dvl.exe cannot be +# found in either location the test fails -- the dvl command-type tests are +# required and silently skipping them would let regressions slip through. +$dvl_exe_path = "C:\Program Files (x86)\Windows Kits\10\Tools\dvl\Dvl.exe" +$nuget_dvl = Get-ChildItem -Path "$starting_location\packages" -Filter "Dvl.exe" ` + -Recurse -ErrorAction SilentlyContinue | Select-Object -First 1 +if ($nuget_dvl) { + $dvl_exe_path = $nuget_dvl.FullName + Write-Host "Using Dvl.exe from NuGet package: $dvl_exe_path" +} elseif (Test-Path $dvl_exe_path) { + Write-Host "Using Dvl.exe from system WDK: $dvl_exe_path" +} else { + Write-Host "FAIL -- Dvl.exe not found in NuGet packages ($starting_location\packages) or system WDK ($dvl_exe_path)." + exit 1 +} function Test-DVL { param ( [string]$command_type = "msbuild", @@ -85,8 +104,7 @@ function Test-DVL { $global:LASTEXITCODE = 1234567891 if ($command_type -eq "dvl") { if ($configuration -eq "Release") { - - $command = "& `"C:\Program Files (x86)\Windows Kits\10\Tools\dvl\Dvl.exe`" /manualCreate $vcxproj_name $platform" + $command = "& `"$dvl_exe_path`" /manualCreate $vcxproj_name $platform" $output = Invoke-Expression $command if ($LastExitCode -eq 1234567891) { Write-Host "FAIL -- Unexpected error creating DVL with $platform $configuration using $command" @@ -174,7 +192,7 @@ function Test-Driver { #Test DVL Test-DVL "msbuild" -test_empty $false - Test-DVL "dvl" -test_emtpy $false + Test-DVL "dvl" -test_empty $false Set-Location -Path $starting_location }