Repository navigation
Conversation
- Keep Core tests buildable without ActiveX - Register ActiveX tests and verify activation test selection - Enforce warnings as errors across configurations - Exercise Win32 Debug in PR CI Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Account for generated MIDL and imported dependency include paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The unlabeled direct-discovery hosting test remains selected by non-interactive runs and, when MWG_TEST_ACTIVEX_HOST is inherited, can activate the wrong registered OCX or fail without the script’s registration setup.
1 open issue
What changed in this PR
This PR advances MapWinGIS6’s CMake migration by separating Core and ActiveX test suites, strengthening activation-test validation, and aligning warning enforcement across build systems.
Changes:
- Adds a dedicated ActiveX CMake test target while keeping Core tests available without ActiveX.
- Rejects activation runs that select no tests and enables
/WXacross configurations and platforms. - Adds Win32 Debug pull-request CI coverage and updates build/testing documentation.
Builds and tests were not run during this review.
| File | Description |
|---|---|
tests/MapWinGIS.Core.Tests/CMakeLists.txt |
Removes ActiveX dependencies and enables full-driver test coverage. |
tests/MapWinGIS.ActiveX.Tests/Verify-ActiveXActivation.ps1 |
Checks the GoogleTest report for selected tests and cleans it up. |
tests/MapWinGIS.ActiveX.Tests/CMakeLists.txt |
Adds the ActiveX test executable, discovery, and interactive activation entry. |
src/MapWinGIS.ActiveX/CMakeLists.txt |
Clarifies a Windows-dependency comment; no functional change. |
README.md |
Documents independent test suites and expanded CI coverage. |
docs/TESTING.md |
Updates CTest and activation-verification guidance. |
docs/CMAKE-MIGRATION.md |
Records separate test targets, dependencies, and warning settings. |
docs/BUILDING.md |
Documents ActiveX CMake tests and Core-only configurations. |
CMakeLists.txt |
Registers Core tests independently and ActiveX tests conditionally. |
cmake/MwgCompilerOptions.cmake |
Enables /WX for all CMake configurations and platforms. |
build/scripts/Compare-CompileCommands.ps1 |
Accounts for ActiveX test include-path differences in parity checks. |
build/props/MapWinGIS.Cpp.props |
Enables warnings-as-errors across MSBuild configurations and platforms. |
AGENTS.md |
Updates validation guidance for CMake’s ActiveX test support. |
.gitignore |
Ignores local tooling and PowerShell editor state. |
.github/workflows/build.yml |
Adds CMake Win32 Debug coverage for pull requests. |
Review effort: Balanced
Julian-MW
reviewed
Nov 13, 2026
Julian-MW
left a comment
There was a problem hiding this comment.
Automated review. Final decision is with the human reviewer.
- Exclude the registered host test from direct CTest discovery - Fail early when shared ActiveX test sources are missing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.

Completes Phase 1 by separating the Core and ActiveX CTest suites, checking that the activation filter selects a test, and enabling
/WXacross configurations and platforms. Adds CMake Win32 Debug coverage to pull-request CI and updates build/testing documentation.Validation:
GdalEnvironment.IncludesTheReleaseDriverSetand the registered-control activation check.Compare-CompileCommands.ps1 -FailOnLostFlags -FailOnNewWarningspasses with zero lost flags and zero CMake warnings. Its incremental MSBuild rebuild emitted twoMSB3061access-denied warnings while replacing runtime DLLs; the standalone MSBuild build reported zero warnings.The review HTML referenced by the plan was unavailable at the checked paths; implementation followed the written Phase 1 criteria and source verification.