From 7c3e97859647e4686113a66716ed4ca885b73589 Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 2 Jun 2026 12:38:16 +0200 Subject: [PATCH 1/2] Discover Copy()-test types by reflection instead of CRDT registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EntityCopyMethodTests drove its theory from crdtConfig.ObjectTypes, the CRDT sync registration. That lists only root entities, so owned types that define their own Copy() (Translation, ViewField, ViewWritingSystem, RichString, RichMultiString, RichSpan) never appeared as direct rows — they were only covered transitively via their owners, and a new model could be forgotten entirely. Enumerate every concrete type in the MiniLcm assembly that declares a parameterless Copy() instead, so coverage tracks the Copy() contract itself and can't drift from a hand-maintained list. Drops the RemoteResource exclusion (it's in the SIL.Harmony assembly, so scanning the MiniLcm assembly excludes it naturally). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../LcmCrdt.Tests/EntityCopyMethodTests.cs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs b/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs index 0548cb0c98..fb8c397b27 100644 --- a/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs @@ -1,7 +1,7 @@ +using System.Reflection; using FluentAssertions.Equivalency; using FluentAssertions.Execution; using MiniLcm.Tests.AutoFakerHelpers; -using SIL.Harmony.Resource; using Soenneker.Utils.AutoBogus; using Soenneker.Utils.AutoBogus.Config; @@ -11,15 +11,21 @@ public class EntityCopyMethodTests { private static readonly AutoFaker AutoFaker = new(AutoFakerDefault.Config); + // Discover types from the Copy() contract itself rather than the CRDT registration, so an + // owned type like Translation (which isn't a CRDT root) — or any model added later — can't slip + // through by being absent from a hand-maintained list. public static IEnumerable GetEntityTypes() { - var crdtConfig = new CrdtConfig(); - LcmCrdtKernel.ConfigureCrdt(crdtConfig); - return crdtConfig.ObjectTypes - .Except([typeof(RemoteResource)])//exclude remote resource as it's a harmony defined type, not miniLcm + return typeof(IObjectWithId).Assembly.GetTypes() + .Where(t => t is { IsClass: true, IsAbstract: false } && CopyMethod(t) is not null) .Select(t => new object[] { t }); } + private static MethodInfo? CopyMethod(Type type) + { + return type.GetMethod("Copy", BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null); + } + private void AssertDeepCopy(object copy, object original) { copy.Should().BeEquivalentTo(original, options => options @@ -48,9 +54,8 @@ public void AssertDeepCopy_FailsForShallowCopy() [MemberData(nameof(GetEntityTypes))] public void EntityCopyMethodShouldCopyAllFields(Type type) { - type.IsAssignableTo(typeof(IObjectWithId)).Should().BeTrue(); - var entity = (IObjectWithId) AutoFaker.Generate(type); - var copy = entity.Copy(); + var entity = AutoFaker.Generate(type)!; + var copy = CopyMethod(type)!.Invoke(entity, null)!; AssertDeepCopy(copy, entity); } From 985d58a3304b1f08ee08a94f2e0a4730d87c690d Mon Sep 17 00:00:00 2001 From: Tim Haasdyk Date: Tue, 2 Jun 2026 12:48:36 +0200 Subject: [PATCH 2/2] Restrict Copy() discovery to declared methods Without BindingFlags.DeclaredOnly the reflection scan also matched types that only inherit Copy(), notably the private RichStringPrimitive helper nested in RichString's JSON converter, which AutoFaker can't construct. DeclaredOnly keeps the 17 types that define their own Copy() and drops inherit-only subclasses. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs b/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs index fb8c397b27..94987fe7d2 100644 --- a/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/EntityCopyMethodTests.cs @@ -23,7 +23,9 @@ public static IEnumerable GetEntityTypes() private static MethodInfo? CopyMethod(Type type) { - return type.GetMethod("Copy", BindingFlags.Public | BindingFlags.Instance, binder: null, Type.EmptyTypes, modifiers: null); + // DeclaredOnly: the type must define its own Copy(), not merely inherit one, so private + // RichString subclasses (e.g. the JSON-converter helper) aren't pulled in as spurious cases. + return type.GetMethod("Copy", BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly, binder: null, Type.EmptyTypes, modifiers: null); } private void AssertDeepCopy(object copy, object original)