Add tests + evaluation notes for the R3 migration sample#555
Merged
Conversation
Add a regression test project for the shipped R3 migration sample, a CI job that runs it, and distil the migration-skill evaluation into dev-docs. - Samples/ReactivePropertySamples.R3.Tests: 15 MSTest tests over the migrated sample ViewModels (validation, commands, POCO sync, EventToReactive, the synchronous collection path); time/thread-dependent VMs are documented as excluded. Added to ReactiveProperty-Samples.slnx under /R3Migration/. - .github/workflows/samples-r3-tests.yml: gating, scoped to the single test project so it stays fast (no WPF/Blazor/Prism/MahApps restore). - dev-docs/r3-migration/skill-evaluation.md: maintainer-facing evaluation (3 findings incl. the fixed YAML frontmatter bug, 51-rule conformance, one trimmed transcript excerpt); linked from ADR-0003 and design.md. - docs/docs/advanced/r3-migration.md: runnable before/after sample pointer and a cross-reference bullet to the existing EnableValidation validation-timing note. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
runceel
added a commit
that referenced
this pull request
Jun 24, 2026
* R3 migration: design doc + remove abandoned compat scaffold (checkpoint) (#540) * Initial plan * Add R3 migration compatibility foundation * Fix compatibility netstandard init support * Address compatibility validation review * Apply final compatibility review fixes * 各プロジェクトで .NET 10 (net10.0) をターゲットに追加 各種 .csproj の TargetFrameworks に net10.0 または net10.0-windows を追加し、ReactiveProperty ライブラリ群が .NET 10 でビルド・利用可能となるよう対応しました。他の設定変更はありません。 * R3 migration: add forward-only design doc; remove abandoned compat scaffold - Add dev-docs/r3-migration/design.md: the forward-only implementation spec for the single, permanent ReactiveProperty.R3 bridge package + migration skill (.NET 10 + C# 14, full per-feature API surface, acceptance criteria). - Remove the abandoned #528 "stopgap" scaffold: the four ReactiveProperty.R3.Compatibility.* projects, the compat test project, and the old migration skill. - Rewire ReactiveProperty.slnx accordingly. Tracked by #528 (sub-issues #533-#539). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Kazuki Ota <117221407+kaota_microsoft@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Raise central C# language version to 14.0 (#541) * Initial plan * Raise C# language version to 14.0 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Scaffold ReactiveProperty.R3 bridge package (#542) * Initial plan * Scaffold ReactiveProperty.R3 package --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Add R3 collection bridge APIs (#543) * Initial plan * Implement R3 collection bridge * Address R3 collection review feedback * Polish R3 collection bridge implementation * Fix value-type Remove reporting and harden property selector parsing (#544) Address two correctness issues found while reviewing the R3 collection bridge (ReactiveProperty.R3): - Remove on value-type elements reported `default(T)` instead of the actual removed element. The converter projection path passed `default(TResult)` to `CollectionChanged<T>.Remove`, which is non-null for value types and therefore surfaced as OldItems. Added a `Remove(int index)` factory (Values = null) so ApplyRemove falls back to the real `_source[index]` value. - ResolvePropertyName accepted nested selectors (`x => x.Child.Name`) and was fragile around casts and explicit names. Rewrote it to: * accept an explicit bare property name when no lambda arrow is present (e.g. propertyName: nameof(Item.Name)), * require exactly one top-level member access for lambda bodies by counting dots at parenthesis/bracket depth 0, so cast receivers such as `((Base)x).Name` are accepted while nested paths are rejected. Adds tests covering value-type Remove, explicit bare property names, and nested-selector rejection. Co-authored-by: Kazuki Ota <117221407+kaota_microsoft@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Kazuki Ota <kaota@microsoft.com> Co-authored-by: Kazuki Ota <117221407+kaota_microsoft@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address R3 bridge review findings: validation consistency, async command null-safety, and synchronized property parsing hardening (#545) * Initial plan * Add R3 bridge gap tests * Implement R3 bridge validation command sync gaps * Address R3 bridge review findings * Polish R3 bridge validation review fixes * Fix R3 review feedback for validation, async command, and property parsing * Fix R3 netstandard2.0 property selector parsing Use netstandard2.0-compatible string parsing APIs and name the unsupported property selector character set. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Modernize R3 collection copies Use collection expressions when copying sequences and arrays in ReactiveProperty.R3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Kazuki Ota <117221407+kaota_microsoft@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * R3 ReactiveTimer/Notifiers: fix IsEnabled parity, deadlock risk, timer leak, and document Interval trade-off (#546) * Initial plan * Add R3 bridge notifiers, ReactiveTimer and message brokers (§4.6-4.8) * Lock-guard ReactiveTimer.Interval to avoid race with timer start * Address review comments: Dispose sets IsEnabled=false, OnNext outside lock, ScheduledNotifier timer holder, CountNotifier clamp test, ADR 0002 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Correct R3 migration skill mapping rules against the actual implementation (#547) * Initial plan * Add migrating-reactiveproperty-to-r3 skill (SKILL.md + rules.json) * Refine rules: consolidate observable-interop rule, drop wildcard catch-all * Address PR #547 review comments on the R3 migration skill --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Add WPF R3 migration sample and finalize R3 migration support (#548) * Initial plan * Add ADR 0003, R3 migration sample, and skill/README updates (S7) * Add WPF R3 migration sample mirroring ReactivePropertySamples.WPF * UI同期対応・マルチTFM化・ビルド安定性向上 - サンプルプロジェクトをnetstandard2.0/net8.0/net9.0/net10.0対応に拡張 - FilteredReadOnlyObservableCollection/AsyncReactiveCommandでSynchronizationContext対応を追加し、UIスレッドでのイベント発火を保証 - 拡張メソッドにSynchronizationContext引数を追加 - XAMLの名前空間typo修正 - 上記機能のテストを追加 - MSBuildターゲットでobjディレクトリ未作成時のエラー回避を追加 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Kazuki Ota <117221407+kaota_microsoft@users.noreply.github.com> * Add WPF EventToReactive behavior helpers for migration support (#550) * Initial plan * Add WPF event behavior support * Stabilize WPF behavior property test * Address WPF behavior review feedback * Guard WPF behavior subscriptions * Replace WPF EventToReactive behaviors with R3 trigger actions The PR previously added Behavior<T> variants of the classic EventToReactive helpers to ReactiveProperty.WPF, which did not address the real migration gap: R3 view models use R3's BindableReactiveProperty<T>/ReactiveCommand, which the classic TriggerActions cannot bind to. - Add new package ReactiveProperty.R3.WPF (Reactive.Bindings.R3.Interactivity) with R3-native EventToReactiveProperty / EventToReactiveCommand trigger actions, IEventToReactiveConverter and ReactiveConverter<T,U> built on R3 Observable<>. - Update the R3 WPF migration sample to use the new helpers (EventTrigger + converters) and drop its code-behind handlers. - Remove the off-target *Behavior additions; revert the classic sample XAML and user docs to their original form. - Replace ADR 0004 with the R3 WPF trigger-action decision and update the index. - Register the new source and test projects in both solutions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs(skill): map WPF EventToReactive helpers to ReactiveProperty.R3.WPF in R3 migration skill The migrating-reactiveproperty-to-r3 skill previously flagged EventToReactiveCommand/EventToReactiveProperty as manual-review with no R3 equivalent. Now that ReactiveProperty.R3.WPF ships R3 trigger actions, add rules (RP-EVENT-TO-RP, RP-EVENT-TO-COMMAND, RP-EVENT-CONVERTER, RP-EVENT-CONVERTER-IFACE) mapping them to Reactive.Bindings.R3.Interactivity, and document the package reference, XAML xmlns swap, and converter OnConvert R3-stream signature change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Kazuki Ota <117221407+kaota_microsoft@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: add R3 migration bridge guidance (#553) Add user-facing documentation for the ReactiveProperty.R3 migration bridge, including installation guidance, namespace mapping, and migration skill usage. Remove the mistaken Samples/R3Migration sample and its stale references. Co-authored-by: Kazuki Ota <117221407+kaota_microsoft@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix R3-migration skill load failure and add validated CLI migration guide (#554) * Fix R3-migration skill load failure and add validated CLI migration guide The migrating-reactiveproperty-to-r3 SKILL.md frontmatter used an unquoted YAML `description:` scalar containing ": " (in "(references/rules.json):"), which YAML parses as a mapping separator. The frontmatter failed to parse and the CLI silently dropped the skill from available skills. Wrap the description in a single-quoted scalar (text unchanged) so it loads. Extend docs/advanced/r3-migration.md with a step-by-step GitHub Copilot CLI migration guide validated end-to-end on a sample WPF app: folder-copy skill install, plan-first then incremental prompts, build/test/fix, and what to expect for manual-review items. Add a note that the ReactiveProperty.R3 bridge packages are not yet published (reference from source meanwhile). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: recommend in-tree .agents/skills install for R3 migration skill Lead the migration-skill install guidance with the project-local .agents/skills/<name>/ path (matches skills/README.md), which keeps references/rules.json inside the working tree so the agent reads it with the structured view tool friction-free (verified: skill auto-loads and VIEW=ok). Keep the personal ~/.copilot/skills/ install as the cross-project alternative and scope the read-scoping note to it. Also harden SKILL.md: tell the agent that references/rules.json ships with the skill and, if a structured read is denied for an out-of-tree install, to read it another way and never report a false content-exclusion/policy block. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Kazuki Ota <117221407+kaota_microsoft@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add tests + evaluation notes for the R3 migration sample (#555) Add a regression test project for the shipped R3 migration sample, a CI job that runs it, and distil the migration-skill evaluation into dev-docs. - Samples/ReactivePropertySamples.R3.Tests: 15 MSTest tests over the migrated sample ViewModels (validation, commands, POCO sync, EventToReactive, the synchronous collection path); time/thread-dependent VMs are documented as excluded. Added to ReactiveProperty-Samples.slnx under /R3Migration/. - .github/workflows/samples-r3-tests.yml: gating, scoped to the single test project so it stays fast (no WPF/Blazor/Prism/MahApps restore). - dev-docs/r3-migration/skill-evaluation.md: maintainer-facing evaluation (3 findings incl. the fixed YAML frontmatter bug, 51-rule conformance, one trimmed transcript excerpt); linked from ADR-0003 and design.md. - docs/docs/advanced/r3-migration.md: runnable before/after sample pointer and a cross-reference bullet to the existing EnableValidation validation-timing note. Co-authored-by: Kazuki Ota <117221407+kaota_microsoft@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Kazuki Ota <117221407+kaota_microsoft@users.noreply.github.com> 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
2 participants
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.
Adds the missing automated safety net for the shipped R3 migration sample, plus the maintainer-facing evaluation evidence and user-doc pointers. This is the upstreamed, deduplicated result of the CLI-driven skill evaluation (the lab
01-before/03-completedsnapshots are intentionally not committed — they largely duplicate the existingReactivePropertySamples.WPF↔ReactivePropertySamples.R3.WPFbefore/after pair; only the genuinely additive parts are upstreamed here).What this adds
1. Sample test project (the real additive value).
Samples/ReactivePropertySamples.R3.Tests/— 15 MSTest cases across 5 files covering the already-shipped migrated ViewModels inReactivePropertySamples.R3.Shared, which previously had zero tests:ValidationViewModelTests(5) — DataAnnotations transitions, eagerValidatableReactiveProperty,IgnoreInitialValidationError,ToReactivePropertyAsSynchronizedtwo-way, SubmitCommand gating.ReactiveCommandViewModelTests(3) —ReactiveCommand,IObservable<bool>-gatedCanExecute,ReactiveCommand<string>parameter.CreateFromPocoViewModelTests(3) — two-way / one-way / one-way-to-source POCO sync.EventToReactiveViewModelTests(2) —OpenFileCommandoutput +CanExecutegate.CollectionsViewModelTests(2) — synchronousReactiveCommand+ObservableCollectionpath. Time/thread-dependent VMs (BasicUsagesDelay,Guidthreadpool,FilteredItemstimer) are excluded by design with a documented file comment.Tests assert validation on transitions to sidestep R3's lazy
EnableValidation(see the evaluation note). CPM (versionlessPackageReference), relativeProjectReference,net10.0(no WPF dependency), file-scoped namespaces, UTF-8 BOM.2. Scoped gating CI.
.github/workflows/samples-r3-tests.ymlrunsdotnet teston just this one project (windows-latest, .NET 10) on PRs tomain/feature/r3-migration-support. Scoping keeps it fast — it does not build the heavy WPF/Blazor/Prism/MahApps samples.3. Maintainer evaluation note.
dev-docs/r3-migration/skill-evaluation.mddistills the CLI-driven skill evaluation: the SKILL.md YAML frontmatter bug (fixed), out-of-treeview-scoping vs. install path, theEnableValidationlazy-validation nuance, and the per-symbol conformance summary. Linked from ADR-0003 anddesign.md. No new ADR.4. User-doc pointers.
docs/docs/advanced/r3-migration.mdgains a "Runnable sample" before/after pointer and a cross-reference to the existing validation-timing note (no duplication).Verification
dotnet test Samples/ReactivePropertySamples.R3.Tests/...→ 15 passed, 0 failed, 0 skipped (independently re-run from a clean checkout).Directory.Packages.propsunchanged (all versions already pinned). NoSource/or existing sample ViewModel files modified — additive only (12 files, +440).Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com