Skip to content

test: Improve unit-test coverage of core asset projects#3232

Draft
sasvdw wants to merge 8 commits into
stride3d:masterfrom
LazyWorksZA:feature/core-asset-test-coverage
Draft

test: Improve unit-test coverage of core asset projects#3232
sasvdw wants to merge 8 commits into
stride3d:masterfrom
LazyWorksZA:feature/core-asset-test-coverage

Conversation

@sasvdw

@sasvdw sasvdw commented Jun 23, 2026

Copy link
Copy Markdown

PR Details

Raises unit-test coverage of the core asset pipeline — Stride.Core.Quantum, Stride.Core.Assets, and Stride.Core.Assets.Quantum — as a regression net for the WPF → Avalonia Game Studio rewrite (cross-platform editor). These are shared, headless projects the editor sits on top of; characterization tests let that refactor be verified as behaviour-preserving (and surface breakages when master is later merged into xplat-editor). No production code is changed — every change is in test projects.

Provenance

The first three commits are cherry-picked verbatim from @Kryptos-FR's feature/assets-unit-tests branch (authorship preserved via -x), as suggested in the issue. As tests inherited from another branch, they were put through a full quality audit before inclusion (below). The remaining commits are new work.

test: Add Stride.Core.Assets.Quantum visitor/collector unit tests        (new)
test: Add Stride.Core.Assets.Quantum override/base-linking unit tests    (new)
test: Add Stride.Core.Assets.Quantum node/registry unit tests            (new)
test: Audit and clean up ported core asset/quantum tests                 (new)
fix:  Adapt ported core tests to current master API                      (new)
[Core.Packages] Add tests to improve coverage    (Nicolas Musset, cherry-picked)
[Core.Assets]   Add tests to improve coverage    (Nicolas Musset, cherry-picked)
[Core.Quantum]  Add tests to improve coverage    (Nicolas Musset, cherry-picked)

Part 1 — Ported Quantum/Assets/Packages tests, rebased and audited

Rebased onto current master. The SDK-style test project (Stride.Build.Sdk.Tests) already provides xunit + Microsoft.NET.Test.Sdk, so the reference branch's redundant package refs were dropped during conflict resolution. One real API-drift break fixed: Package.IsSystem was removed upstream.

Quality audit of the 42 ported files (+204 / −896 lines), goal-driven (coverage for refactor safety — keep trivial-but-real tests, only remove tests that detect nothing):

  • Removed ~40 no-value tests: tautologies (Assert.Equal(x, x), Assert.NotNull(new Foo())), framework-tests (C# try/catch, reflection "method exists"), pure duplicates.
  • Strengthened ~20 vacuous/mislabeled tests that were genuinely wrong: AssetItem.IsDirty only tested true→true; Package.Meta.Dependencies had a dead-code branch; NugetPackageBuilder populate asserted the input not the builder; GetHashCode tests checked only call-determinism; ToString tests now assert exact documented formats.
  • Merged near-duplicates into [Theory] cases; cleaned redundant usings; made TestPackageFile cross-platform; dropped environment-coupled PackageStore lookups that write to the user's NuGet config (kept the deterministic null-guard).

Part 2 — New Stride.Core.Assets.Quantum tests (13 files, 3 commits)

The reference branch skipped this project. It has solid integration coverage (archetypes, reconcile, override serialization) but its building blocks lacked direct unit tests:

  • Node / registry layer: AssetNodeContainer.IsPrimitiveType (leaf-vs-expand filter for engine value types, incl. nullable unwrap), AssetNodeFactory (asset-aware node types incl. boxed structs), AssetQuantumRegistry (definition base-walk + Asset fallback + caching + guards; graph construction), AssetPropertyGraphContainer (graph register/lookup/unregister + propagate flag).
  • Override / base-linking layer: member override state (BaseNew, stops following base once overridden, resets to inherited), base-to-derived BaseNode linkage, collection ItemId↔index stability and deletion-as-override, IAssetNode attached-content side-channel.
  • Visitor layer: IdentifiableObjectCollector (owned vs. referenced), ClearObjectReferenceVisitor (clear by id + predicate), ExternalReferenceCollector (external vs. internal), and the save-time OverrideTypePathGenerator / ObjectReferencePathGenerator.

Test names and comments are intentionally behaviour-descriptive so the suite doubles as documentation of how the asset/override/reference pipeline works.

Verification

Built and ran all three suites against current master with the xunit runner:

Project Build Tests
Stride.Core.Quantum.Tests 0 errors 161 passed, 0 failed, 25 skipped*
Stride.Core.Assets.Tests 0 errors 363 passed, 0 failed, 5 skipped*
Stride.Core.Assets.Quantum.Tests 0 errors 170 passed, 0 failed, 1 skipped*

* skips are pre-existing [Fact(Skip=…)] tests (TestDynamicNode/obsolete/stub), not introduced here.

Code coverage (line coverage)

No production code changes, so coverable lines are identical before/after (16,692 across the three assemblies); only covered lines move. Measured with the .NET coverage collector against the same binaries:

Assembly Before (master) After (this PR) Δ
Stride.Core.Quantum 67.9% 77.1% +9.2
Stride.Core.Assets 41.1% 44.1% +3.0
Stride.Core.Assets.Quantum 66.0% 69.3% +3.3
Aggregate (3 assemblies) 47.5% 51.2% +3.7

Covered lines: 7,930 → 8,561 (+631). Test count: Quantum 84→161, Assets 86→363, Assets.Quantum 97→170.

Reproducing the coverage numbers

One-time tooling (global tools, no project changes):

dotnet tool install --global dotnet-coverage
dotnet tool install --global dotnet-reportgenerator-globaltool

After (this branch), from the repo root:

dotnet build sources/presentation/Stride.Core.Quantum.Tests/Stride.Core.Quantum.Tests.csproj -c Debug
dotnet build sources/assets/Stride.Core.Assets.Tests/Stride.Core.Assets.Tests.csproj -c Debug
dotnet build sources/assets/Stride.Core.Assets.Quantum.Tests/Stride.Core.Assets.Quantum.Tests.csproj -c Debug

dotnet-coverage collect -o cov_quantum.coverage       "dotnet test sources/presentation/Stride.Core.Quantum.Tests/Stride.Core.Quantum.Tests.csproj -c Debug --no-build"
dotnet-coverage collect -o cov_assets.coverage        "dotnet test sources/assets/Stride.Core.Assets.Tests/Stride.Core.Assets.Tests.csproj -c Debug --no-build"
dotnet-coverage collect -o cov_assetsquantum.coverage "dotnet test sources/assets/Stride.Core.Assets.Quantum.Tests/Stride.Core.Assets.Quantum.Tests.csproj -c Debug --no-build"

dotnet-coverage merge -f cobertura -o after.cobertura.xml cov_*.coverage
reportgenerator -reports:after.cobertura.xml -targetdir:after-report -reporttypes:TextSummary \
  "-assemblyfilters:+Stride.Core.Quantum;+Stride.Core.Assets;+Stride.Core.Assets.Quantum"
cat after-report/Summary.txt

Before (baseline): run the identical block in a clean git worktree of master, e.g. git worktree add ../stride-baseline origin/master. Merging the three per-project runs is what gives each production assembly its full number (each is exercised by more than one test project).

Limitations

  • The ported tests are characterization tests — they pin current behaviour (including a couple of documented quirks), which is appropriate for a refactor net.
  • Coverage is breadth-first on logic-bearing types, not line-coverage-complete. The Assets.Quantum base-to-derived registry internals and deep reconciliation remain covered by the existing integration tests rather than new micro-tests.

Related Issue

Closes #2749

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have built and run the editor to try this change out.

Kryptos-FR and others added 4 commits June 22, 2026 15:56
The cherry-picked test commits predate master by ~6 months. Remove
TestPackageBasics.TestIsSystemProperty, which exercised the removed
Package.IsSystem property (the test was also a tautology).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

🤖 Draft PR — automatic CI is skipped to save runner minutes.

  • Mark the PR ready for review to run the full automatic CI — or add a ci-run-on-draft label to run it now without leaving draft.
  • Or arm a specific opt-in suite: ci-enduser, ci-editor, ci-ios, ci-android.

@sasvdw

sasvdw commented Jun 23, 2026

Copy link
Copy Markdown
Author

@dotnet-policy-service agree

@Kryptos-FR

Copy link
Copy Markdown
Member

Thanks @sasvdw for the work. Can you reword the PR description to follow the template? Since you are using Claude, it should be able to do it easily.

@Kryptos-FR Kryptos-FR added area-Asset area-Core Issue of the engine unrelated to other defined areas labels Jun 23, 2026
@sasvdw

sasvdw commented Jun 23, 2026

Copy link
Copy Markdown
Author

My apologies. I was going to do the rest of the PR admin once I'm happy with the changes. Hopefully I'lll have some time tonight to sift through everything and validate that the test suite is sensible.

sasvdw and others added 4 commits June 23, 2026 17:05
The cherry-picked tests carried recurring cruft.
This pass tightens them into a meaningful regression net for the
upcoming editor rewrite refactors:

- Remove tautological, framework-only, and pure-duplicate tests that
  detect no regression (Assert.Equal(x, x), reflection "method exists"
  checks, C# try/catch tests, NotNull on freshly-constructed objects).
- Strengthen vacuous/mislabeled tests to assert real behavior, e.g.
  AssetItem.IsDirty false->true + ModifiedTime, Package.Meta.Dependencies
  actual (null) state, NugetPackageBuilder population, GetHashCode equal-
  objects-equal-hash, and exact ToString/format-string assertions.
- Merge near-duplicate cases into [Theory] data cases.
- Make TestPackageFile stream test cross-platform (ThrowsAny<IOException>).
- Drop environment-coupled PackageStore lookups that mutate the user's
  NuGet config; keep the deterministic null guard.
- Remove redundant `using Xunit;`/`using Stride.Core.Assets;` (global
  usings in Assets.Tests) and unused usings; suppress intentional CS1718.

All tests pass: Quantum 161, Assets 363.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Direct unit coverage for the building blocks of the asset property-graph
layer, which were previously only exercised indirectly through the
archetype/reconcile integration tests:

- AssetNodeContainer.IsPrimitiveType: the filter deciding which engine
  value types (Vector3, AssetId, UPath, content references, ...) stay
  graph leaves vs. expand; includes nullable unwrapping.
- AssetNodeFactory: graph nodes are asset-aware (AssetObjectNode /
  AssetMemberNode / AssetBoxedNode for boxed structs).
- AssetQuantumRegistry: GetDefinition base-type walk + Asset fallback +
  caching + non-asset guard; ConstructPropertyGraph type resolution.
- AssetPropertyGraphContainer: graph register/lookup/unregister lifecycle
  and the PropagateChangesFromBase flag.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Direct unit coverage for the inheritance machinery that the editor's
"inherited vs. overridden" UI and reconciliation rely on:

- AssetMemberNode override state: a derived member starts inherited
  (OverrideType.Base), becomes OverrideType.New when written, stops
  following base changes once overridden, and re-inherits after
  ResetOverrideRecursively.
- Base-to-derived linking: derived nodes expose BaseNode pointing at the
  matching base graph node (and null when not derived).
- Collection item identity (IAssetObjectNode): stable ItemId<->index
  mapping that stays bound to values across removal, and removing an
  inherited item being tracked as a deletion override.
- IAssetNode SetContent/GetContent attached-content side-channel.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Direct unit coverage for the graph-visitor layer that powers object
reference handling, cloning and save-time metadata generation:

- IdentifiableObjectCollector: collects owned identifiables while
  ignoring referenced-only ones, plus argument validation.
- ClearObjectReferenceVisitor: clears references to a target id,
  honours the shouldClearReference predicate, leaves other ids intact.
- ExternalReferenceCollector: classifies references as external vs.
  internal (an owned+referenced object is internal) and records accessors.
- OverrideTypePathGenerator / ObjectReferencePathGenerator (via the
  AssetPropertyGraph.Generate* save helpers): emit exactly the overridden
  members and object-reference target ids, nothing for inherited/owned.

Full Stride.Core.Assets.Quantum.Tests suite: 170 passed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@sasvdw sasvdw force-pushed the feature/core-asset-test-coverage branch from 99b5db1 to cb6ae45 Compare June 23, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Asset area-Core Issue of the engine unrelated to other defined areas

Projects

Development

Successfully merging this pull request may close these issues.

❕🟠𝐋 Improve test coverage of core asset projects

2 participants