Skip to content

Add pin tests for gaps found in downstream usage - #78

Merged
jbtule merged 2 commits into
masterfrom
add-pin-tests-impromptu-usage
Sep 22, 2026
Merged

jbtule merged 2 commits into
masterfrom
add-pin-tests-impromptu-usage

Conversation

@jbtule

@jbtule jbtule commented Sep 22, 2026

Copy link
Copy Markdown
Member

Why

Auditing a large downstream consumer (a separate private solution) for how it uses ImpromptuInterface surfaced a few real API usage patterns that aren't exercised anywhere in this test suite. This PR adds pin tests for those specific gaps only; naming is intentionally generic (no downstream business terms).

Gaps covered

  1. Impromptu.DynamicActLike(object, params Type[]) (the non-generic, runtime-Type overload) has no existing test coverage anywhere in the suite. Added a basic round-trip test plus a repeated-call test with the same runtime Type.
  2. ActLike<IList<T>> (a generic BCL collection interface) proxied over an object that only implements the dynamic-dispatch protocol (TryGetMember/TryInvokeMember/TryGetIndex/TrySetIndex), simulating lazy-materialized proxies (e.g. Dynamitey.DynamicObjects.Lazy). Only a non-generic array/indexer case (IStringIntIndexer) was previously tested; this exercises Count, indexer get, Contains, Add, and GetEnumerator() all being synthesized by the proxy, plus confirms the lazy factory only runs once.
  3. Combining a proxy interface with an empty marker/tag interface via the typeof(...) overload. DoubleInterfacetest (existing) only combines two interfaces that both declare members; this adds coverage for a marker interface with zero members.

Validation

dotnet test .\Tests\UnitTestImpromptuInterface\UnitTestImpromptuInterface.csproj -c Debug -f net8.0

All 207 tests pass (203 existing + 4 new).

…ver lazy dynamic objects, and empty marker-interface combination

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings September 22, 2026 20:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Add PinTests.cs to the WASM test project so these tests run in the browser-WASM test leg.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds four focused pin tests for runtime interface proxying, lazy dynamic collections, and marker-interface composition.

Changes:

  • Tests runtime-Type DynamicActLike calls.
  • Tests lazy dynamic IList<int> proxy behavior.
  • Tests combining a proxy with an empty marker interface.
File Description
Tests/​UnitTestImpromptuInterface/​PinTests.cs Adds four targeted tests and supporting proxy types; the file is not yet included in the WASM test project.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Tests/UnitTestImpromptuInterface/PinTests.cs Outdated
- DynamicActLikeWithRuntimeTypeRepeatedTest asserted only the two names,
  which the unwrapped ExpandoObject answers just as well: replacing the
  whole entry point with `=> originalDynamic` left it green. It now
  asserts each result is a proxy, that they are distinct instances, and
  that they share a proxy type - the reuse its comment claimed and
  nothing checked. The same mutation now fails it.
- Count the factory's calls in the factory. FactoryCallCount++ sat behind
  the `_target == null` guard, so it could never exceed 1 whatever the
  proxy did, and asserting it was 1 pinned nothing.
- Exercise TrySetIndex, which the fixture implements and no test reached.
- PinTests.cs is linked into the wasm project, which lists its sources
  explicitly. It pulls in neither Moq nor IronPython, the two documented
  exclusions, and it is the only generic-collection-over-DynamicObject
  case that leg gets - the comparable ones are in the excluded Linq.cs.
- The header and the "would go in SupportDefinitions" note read as a
  proposal to a reviewer rather than comments in a committed file; the
  runtime-Type gap is the public static entry point, not the behaviour,
  which ActLikeCaster covers heavily. Unused using removed.

207 on net8.0 and net10.0, 213 on net47, 118 on browser-wasm.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016mwfq4oeZW8SiD4HjTHYdg
@jbtule

jbtule commented Sep 22, 2026

Copy link
Copy Markdown
Member Author

That one is right, and it is fixed in a323270 — PinTests.cs is now linked into the wasm project, which takes that leg from 114 to 118 tests, all passing (bun runtests.mjs).

It belongs there rather than being excluded: the file pulls in neither Moq nor IronPython, which are the two documented reasons SingleMethodInvoke.cs and Linq.cs stay out. And it is worth having in particular, because GenericListInterfaceOverLazyDynamicTest is the only generic-collection-over-DynamicObject proxy case that leg gets — the comparable existing ones live in the excluded Linq.cs.

A cold review of the same commit turned up four other things, all in a323270 as well:

  • DynamicActLikeWithRuntimeTypeRepeatedTest did not pin its own API. Replacing the whole entry point with public static dynamic DynamicActLike(object o, params Type[] t) => o; left it green, because reading .Name off the unwrapped ExpandoObject gives the same answer. It now asserts each result is a proxy, that they are distinct instances, and that they share a proxy type — the reuse its comment claimed and nothing checked. Re-ran that same mutation afterwards: it now fails both tests rather than one.
  • Assert.AreEqual(1, tForwarder.FactoryCallCount) could not fail. The increment sat behind the fixture's own _target == null guard, so the count could never exceed 1 whatever the proxy did. The factory now counts its own calls, which pins the property worth pinning: the proxy does not re-materialize the target per member call.
  • TrySetIndex was implemented in the fixture and reached by no test, though the description listed it. Now exercised through the synthesized IList<int> setter.
  • Header comments addressed a reviewer of a proposal ("can live directly in Tests/…", "would go in Support/SupportDefinitions.cs") rather than describing the file; and the runtime-Type gap is the public static entry point, not the behaviour — no-opping ActLikeMaker.DynamicActLike turns 17 existing tests red.

207 on net8.0 and net10.0, 213 on net47, 118 on browser-wasm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants