Conversation
When AssemblyInitialize throws (e.g. the test web app never becomes reachable), MSTest skips AssemblyCleanup, so no FINAL_SUMMARY line is written and GBTest read total=0. The pass percentage then defaulted to 100, reporting a green run even though zero tests ran. Treat a missing summary / total=0 as a failure, and treat a cancelled run as a failure as well. Rebuilt GBTest.dll for all platforms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Root cause of the ubuntu-latest test failure: StartWebApp launches the web app with 'dotnet run --no-build', but nothing in the test pipeline builds the web app (the Automation project does not reference it). On a clean runner the launch had no assembly to run, so the readiness poll timed out after 15 minutes with a misleading 'page not reachable' error. Fixes: - Build the test web app explicitly in StartWebApp before the --no-build launch, applying the same MSBuild properties (incl. ShowScriptDialogs=false). The run's /p: properties are passed after '--' so they reach the app, not a build - hence a dedicated build step rather than dropping --no-build. - Set _webTestProcessId in the non-container path so WaitForHttpResponse detects an early process exit instead of polling to timeout (it was only set in the container path). - Fix WaitForHttpResponse swallowing the non-zero-exit ProcessExitedException: the throw was inside the try and caught by the bare catch. Read ExitCode defensively, throw outside. - Improve the failure messages to point at the WEB_APP_SERVER / WEB_APP_ERROR log output. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The previous fix built the web app in Release, but 'dotnet run --no-build' had -c Release only after '--' (an app arg), so it defaulted to Debug and looked in bin/Debug -> 'No such file', never starting -> 15m poll x3 Polly retries (~1h20m). Core unit tests hit the same class of issue: launched with 'dotnet test --no-build' but never built. - Web app run: move -c <config> before '--' so --no-build resolves bin/Release. - Add EnsureTestProjectBuilt: build a test project once, in _runConfig, before its --no-build launch. Used by both StartWebApp and RunUnitTests (fixes CORE_UNIT/PRO_VALIDATION 'No such file or directory'). - Serialize these builds via a semaphore: the web app and unit-test tasks run in parallel, so building concurrently raced on the shared ESBuild lock (AcquireBuildLock exit code 1). - Build-once guard avoids rebuilding on Polly retries. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
After the build/launch fixes, all 405 web tests ran but failed, and the cause was invisible: the FINAL_SUMMARY 'FAILED TEST DETAILS' section was blank. Diagnostics: - GeoBlazorTestClass: capture the actual exception (type/message/stack) for a failed web test, not just browser console text (which is empty when the page fails before logging). - TestConfig summary: replace the hard Substring(26) - which threw on short messages like 'Test Failed' and, with no catch in AssemblyCleanup, aborted the whole details section - with a guarded prefix strip, wrapped in try/catch so one bad entry can't blank the report. Likely root cause fix: - Inject the ArcGIS API key into the launched web app as the 'ArcGISApiKey' env var (ASP.NET maps it onto the config key, overriding the appsettings.Development.json placeholder). CI provides ARCGIS_API_KEY but nothing plumbed it into the web app; only the license was injected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
With per-test errors now visible, all 405 web tests were failing identically: BuildTestUrl interpolated the BlazorMode enum, and Enum.ToString -> EnumInfo.Create threw BadImageFormatException / CLDB_E_INDEX_NOTFOUND (corrupt enum name metadata in the automation process) on every test. Map the enum to its name via a switch of nameof constants, which compiles to integer comparisons and needs no reflection metadata. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Make MapComponent.GetLayer virtual and add the missing UpdateGeoBlazorReferences call so it matches the per-component versions being consolidated. - Remove the now-redundant generated GetLayer() overrides from inherited components (base provides them); keep Graphic.GetLayer as an override since it returns Parent rather than querying JS. - Add a one-shot SetLayer trigger to MapComponent.OnAfterRenderAsync so a layer added to the map after a component is built (id-binding pattern) is pushed to JS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Notes for reviewers (not part of release notes):
|
Contributor
There was a problem hiding this comment.
Pull request overview
Release v4.6.0 update focused on fixing geometry operator results (notably Union extent population), improving layer-by-id late binding, and hardening the automation test/CI harness so failures surface correctly and reliably.
Changes:
- Fix geometry deserialization/interop so operator results return usable geometries with populated
Extent. - Centralize
GetLayer()/SetLayer()behavior (JS + .NET) and improve late layer resolution syncing. - Improve automation harness reliability: pre-build projects for
--no-buildruns, inject ArcGIS API key for web tests, and improve failure reporting and “no tests ran” detection.
Reviewed changes
Copilot reviewed 31 out of 40 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/dymaptic.GeoBlazor.Core.Test.Blazor.Shared/Components/WebMapTests.razor | Excludes a web map test from CI execution. |
| test/dymaptic.GeoBlazor.Core.Test.Automation/TestConfig.cs | Adds build-serialization and pre-build steps; improves logging, API key injection, and fail-fast behaviors. |
| test/dymaptic.GeoBlazor.Core.Test.Automation/GeoBlazorTestClass.cs | Improves web-test failure details and avoids enum reflection in URL building. |
| src/dymaptic.GeoBlazor.Core/Serialization/GeometryConverter.cs | Populates Geometry.Extent from operator results during deserialization. |
| src/dymaptic.GeoBlazor.Core/Scripts/wFSLayerView.gb.ts | Removes per-class getLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/sublayer.gb.ts | Removes per-class getLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/listItem.gb.ts | Removes per-class getLayer()/setLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/kMLLayerView.gb.ts | Removes per-class getLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/imageryTileLayerView.gb.ts | Removes per-class getLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/imageryLayerView.gb.ts | Removes per-class getLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/graphicsLayerView.gb.ts | Removes per-class getLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/geoRSSLayerView.gb.ts | Removes per-class getLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/geoJSONLayerView.gb.ts | Removes per-class getLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/featureLayerView.gb.ts | Removes per-class getLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/cSVLayerView.gb.ts | Removes per-class getLayer() in favor of base implementation. |
| src/dymaptic.GeoBlazor.Core/Scripts/baseComponent.ts | Adds shared JS getLayer()/setLayer() on the base wrapper. |
| src/dymaptic.GeoBlazor.Core/Scripts/arcGisJsInterop.ts | Switches geometry engine wrapper to return .NET-friendly results. |
| src/dymaptic.GeoBlazor.Core/Components/MapComponent.razor.cs | Adds base GetLayer()/SetLayer() and late layer-to-JS sync behavior. |
| src/dymaptic.GeoBlazor.Core/Components/ListItem.gb.cs | Removes duplicated GetLayer() from generated component. |
| src/dymaptic.GeoBlazor.Core/Components/LegendViewModelLayerInfos.gb.cs | Removes duplicated GetLayer() from generated component. |
| src/dymaptic.GeoBlazor.Core/Components/LegendViewModelLayerInfo.gb.cs | Removes duplicated GetLayer() from generated component. |
| src/dymaptic.GeoBlazor.Core/Components/LegendLayerInfos.gb.cs | Removes duplicated GetLayer() from generated component. |
| src/dymaptic.GeoBlazor.Core/Components/LayerSearchSource.gb.cs | Removes duplicated GetLayer() from generated component. |
| src/dymaptic.GeoBlazor.Core/Components/Graphic.cs | Makes Graphic.GetLayer() correctly override the base virtual. |
| src/dymaptic.GeoBlazor.Core/Components/FeatureSnappingLayerSource.gb.cs | Removes duplicated GetLayer() from generated component. |
| src/dymaptic.GeoBlazor.Core/Components/ActiveLayerInfo.gb.cs | Removes duplicated GetLayer() from generated component. |
| Directory.Build.props | Bumps CoreVersion to 4.6.0. |
| build-tools/win-x64/GBTest.runtimeconfig.json | Updates test runner runtimeconfig (currently with machine-specific entrypoint paths). |
| build-tools/osx-arm64/GBTest.runtimeconfig.json | Updates test runner runtimeconfig (currently with machine-specific entrypoint paths). |
| build-tools/linux-x64/GBTest.runtimeconfig.json | Updates test runner runtimeconfig (currently with machine-specific entrypoint paths). |
| build-tools/build-scripts/GBTest.cs | Makes GBTest fail on cancellation and correctly fail when no tests ran / no summary produced. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Release v4.6.0
🐛 Bug Fixes
GeometryEngine.Unionreturns a populated result again. A regression in 4.5.1 causedUnionto come back without itsExtent(and in some paths asnull).Unionnow returns the full unioned geometry with a populatedExtent, so calls likeview.GoTo(union.Extent)work as expected.SketchWidgetwithLayerId/GraphicsLayerId) now correctly resolve the layer and push it to the underlying ArcGIS JS object when the layer is added to the map after the component first renders.✨ New Features
GetLayer()/SetLayer()onMapComponent. These are now available on theMapComponentbase class, so any layer-bearing component can retrieve or set its layer at runtime (previously they were implemented per-component).🧰 Internal / Build / Tooling
--no-buildlaunches, surface web-test failures, inject the ArcGIS API key into the test web app, fail the run when no tests execute, and avoid enum reflection when building test URLs.