Skip to content

Commit 0bacb18

Browse files
myieyemyieye
authored
Make CanSyncRandomEntries more robust (#2075)
* Don't generate redundant multi-string values * Stablize fwdata complex-form and component headwords * Make CanSyncRandomEntries robuster - Runs against FwApi as well - Include moves and positional creates (components, example, complex-forms) - Round-trip data through each api so it's realistic * Add GitHub link to headword todo * Add another comment mentioning poor headword handling * Fix async syntax warning * Make expected a clone to ensure it's stable. * Thoroughly explain the motivation for the test
1 parent 5000153 commit 0bacb18

File tree

7 files changed

+336
-133
lines changed

7 files changed

+336
-133
lines changed

‎backend/FwLite/FwDataMiniLcmBridge/Api/LcmHelpers.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,22 @@ internal static class LcmHelpers
1515
{
1616
var citationFormTs =
1717
ws.HasValue ? entry.CitationForm.get_String(ws.Value)
18-
: entry.CitationForm.StringCount > 0 ? entry.CitationForm.GetStringFromIndex(0, out var _)
18+
: entry.CitationForm.StringCount > 0 ? entry.CitationForm.BestVernacularAlternative
1919
: null;
2020
var citationForm = citationFormTs?.Text?.Trim(WhitespaceChars);
2121

2222
if (!string.IsNullOrEmpty(citationForm)) return citationForm;
2323

2424
var lexemeFormTs =
2525
ws.HasValue ? entry.LexemeFormOA?.Form.get_String(ws.Value)
26-
: entry.LexemeFormOA?.Form.StringCount > 0 ? entry.LexemeFormOA?.Form.GetStringFromIndex(0, out var _)
26+
: entry.LexemeFormOA?.Form.StringCount > 0 ? entry.LexemeFormOA?.Form.BestVernacularAlternative
2727
: null;
2828
var lexemeForm = lexemeFormTs?.Text?.Trim(WhitespaceChars);
2929

3030
return lexemeForm;
3131
}
3232

33-
internal static string? LexEntryHeadwordOrUnknown(this ILexEntry entry, int? ws = null)
33+
internal static string LexEntryHeadwordOrUnknown(this ILexEntry entry, int? ws = null)
3434
{
3535
var headword = entry.LexEntryHeadword(ws);
3636
return string.IsNullOrEmpty(headword) ? Entry.UnknownHeadword : headword;

‎backend/FwLite/FwLiteProjectSync.Tests/EntrySyncTests.cs‎

Lines changed: 195 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using System.Text;
2+
using FwDataMiniLcmBridge.Api;
23
using FwLiteProjectSync.Tests.Fixtures;
4+
using LcmCrdt;
35
using MiniLcm;
46
using MiniLcm.Models;
57
using MiniLcm.SyncHelpers;
@@ -8,53 +10,50 @@
810

911
namespace FwLiteProjectSync.Tests;
1012

11-
public class CrdtEntrySyncTests(SyncFixture fixture) : EntrySyncTestsBase(fixture)
13+
public class CrdtEntrySyncTests(ExtraWritingSystemsSyncFixture fixture) : EntrySyncTestsBase(fixture)
1214
{
13-
private static readonly AutoFaker AutoFaker = new(AutoFakerDefault.Config);
14-
1515
protected override IMiniLcmApi GetApi(SyncFixture fixture)
1616
{
1717
return fixture.CrdtApi;
1818
}
19-
20-
[Fact]
21-
public async Task CanSyncRandomEntries()
22-
{
23-
var createdEntry = await Api.CreateEntry(await AutoFaker.EntryReadyForCreation(Api));
24-
var after = await AutoFaker.EntryReadyForCreation(Api, entryId: createdEntry.Id);
25-
26-
after.Senses = [.. AutoFaker.Faker.Random.Shuffle([
27-
// copy some senses over, so moves happen
28-
..AutoFaker.Faker.Random.ListItems(createdEntry.Senses),
29-
..after.Senses
30-
])];
31-
32-
await EntrySync.SyncFull(createdEntry, after, Api);
33-
var actual = await Api.GetEntry(after.Id);
34-
actual.Should().NotBeNull();
35-
actual.Should().BeEquivalentTo(after, options => options
36-
.For(e => e.Senses).Exclude(s => s.Order)
37-
.For(e => e.Components).Exclude(c => c.Order)
38-
.For(e => e.ComplexForms).Exclude(c => c.Order)
39-
.For(e => e.Senses).For(s => s.ExampleSentences).Exclude(e => e.Order)
40-
);
41-
}
4219
}
4320

44-
public class FwDataEntrySyncTests(SyncFixture fixture) : EntrySyncTestsBase(fixture)
21+
public class FwDataEntrySyncTests(ExtraWritingSystemsSyncFixture fixture) : EntrySyncTestsBase(fixture)
4522
{
4623
protected override IMiniLcmApi GetApi(SyncFixture fixture)
4724
{
4825
return fixture.FwDataApi;
4926
}
27+
28+
// this will notify us when we start syncing MorphType (if that ever happens)
29+
[Fact]
30+
public async Task FwDataApiDoesNotUpdateMorphType()
31+
{
32+
// arrange
33+
var entry = await Api.CreateEntry(new()
34+
{
35+
LexemeForm = { { "en", "morph-type-test" } },
36+
MorphType = MorphType.BoundStem
37+
});
38+
39+
// act
40+
var updatedEntry = entry.Copy();
41+
updatedEntry.MorphType = MorphType.Suffix;
42+
await EntrySync.SyncFull(entry, updatedEntry, Api);
43+
44+
// assert
45+
var actual = await Api.GetEntry(entry.Id);
46+
actual.Should().NotBeNull();
47+
actual.MorphType.Should().Be(MorphType.BoundStem);
48+
}
5049
}
5150

52-
public abstract class EntrySyncTestsBase(SyncFixture fixture) : IClassFixture<SyncFixture>, IAsyncLifetime
51+
public abstract class EntrySyncTestsBase(ExtraWritingSystemsSyncFixture fixture) : IClassFixture<ExtraWritingSystemsSyncFixture>, IAsyncLifetime
5352
{
54-
public async Task InitializeAsync()
53+
public Task InitializeAsync()
5554
{
56-
await _fixture.EnsureDefaultVernacularWritingSystemExistsInCrdt();
5755
Api = GetApi(_fixture);
56+
return Task.CompletedTask;
5857
}
5958

6059
public Task DisposeAsync()
@@ -67,6 +66,172 @@ public Task DisposeAsync()
6766
private readonly SyncFixture _fixture = fixture;
6867
protected IMiniLcmApi Api = null!;
6968

69+
private static readonly AutoFaker AutoFaker = new(AutoFakerDefault.MakeConfig(
70+
ExtraWritingSystemsSyncFixture.VernacularWritingSystems));
71+
72+
public enum ApiType
73+
{
74+
Crdt,
75+
FwData
76+
}
77+
78+
// The round-tripping api is not what is under test here. It's purely for preprocessing.
79+
// It's so that that the data under test is being read from a real API (i.e. fwdata or crdt)
80+
// and thus reflects whatever nuances that API may have.
81+
//
82+
// Not all of the test cases are realistic, but they should all work and they reflect the idea
83+
// that "any MiniLcmApi implementation should be compatible with any other implementation".
84+
// Even the unrealistic test cases could potentially expose unexpected, undesirable nuances in API behaviour.
85+
// They also reflect the diversity of pipelines real entries might go through.
86+
// For example, a currently real scenario is that "after" is read from fwdata and "before" is read from crdt
87+
// and then round-tripped through a json file.
88+
// That case is not explicitly covered here.
89+
//
90+
// The most critical test cases are:
91+
// Api == CrdtApi and RoundTripApi == FwDataApi
92+
// Api == FwDataApi and RoundTripApi == CrdtApi
93+
// (though, as noted above, this case doesn't perfectly reflect real usage)
94+
[Theory]
95+
[InlineData(ApiType.Crdt)]
96+
[InlineData(ApiType.FwData)]
97+
[InlineData(null)]
98+
public async Task CanSyncRandomEntries(ApiType? roundTripApiType)
99+
{
100+
// arrange
101+
var currentApiType = Api switch
102+
{
103+
FwDataMiniLcmApi => ApiType.FwData,
104+
CrdtMiniLcmApi => ApiType.Crdt,
105+
// This works now, because we're not currently wrapping Api,
106+
// but if we ever do, then we want this to throw, so we know we need to detect the api differently.
107+
_ => throw new InvalidOperationException("Unknown API type")
108+
};
109+
110+
IMiniLcmApi? roundTripApi = roundTripApiType switch
111+
{
112+
ApiType.Crdt => _fixture.CrdtApi,
113+
ApiType.FwData => _fixture.FwDataApi,
114+
_ => null
115+
};
116+
117+
var before = AutoFaker.Generate<Entry>();
118+
var after = AutoFaker.Generate<Entry>();
119+
after.Id = before.Id;
120+
121+
// We have to "prepare" while before and after have no overlap (i.e. before we start mixing parts of before into after),
122+
// otherwise "PrepareToCreateEntry" would fail due to trying to create duplicate related entities.
123+
// After this we can't ADD anything to after that has dependencies
124+
// e.g. ExampleSentences are fine, because they're owned/part of an entry.
125+
// Parts of speech, on the other hand, are not owned by an entry.
126+
await Api.PrepareToCreateEntry(before);
127+
await Api.PrepareToCreateEntry(after);
128+
129+
if (roundTripApi is not null && currentApiType != roundTripApiType)
130+
{
131+
await roundTripApi.PrepareToCreateEntry(before);
132+
await roundTripApi.PrepareToCreateEntry(after);
133+
}
134+
135+
// keep some old senses, remove others
136+
var someRandomBeforeSenses = AutoFaker.Faker.Random.ListItems(before.Senses).Select(createdSense =>
137+
{
138+
var copy = createdSense.Copy();
139+
copy.ExampleSentences = [
140+
// shuffle to cause moves
141+
..AutoFaker.Faker.Random.Shuffle([
142+
// keep some, remove others
143+
..AutoFaker.Faker.Random.ListItems(copy.ExampleSentences),
144+
// add new
145+
AutoFaker.ExampleSentence(copy),
146+
AutoFaker.ExampleSentence(copy),
147+
]),
148+
];
149+
return copy;
150+
});
151+
// keep new, and shuffle to cause moves
152+
after.Senses = [.. AutoFaker.Faker.Random.Shuffle([.. someRandomBeforeSenses, .. after.Senses])];
153+
154+
after.ComplexForms = [
155+
// shuffle to cause moves
156+
..AutoFaker.Faker.Random.Shuffle([
157+
// keep some, remove others
158+
..AutoFaker.Faker.Random.ListItems(before.ComplexForms)
159+
.Select(createdCfc =>
160+
{
161+
var copy = createdCfc.Copy();
162+
copy.ComponentHeadword = after.Headword();
163+
return copy;
164+
}),
165+
// keep new
166+
..after.ComplexForms
167+
]),
168+
];
169+
170+
after.Components = [
171+
// shuffle to cause moves
172+
..AutoFaker.Faker.Random.Shuffle([
173+
// keep some, remove others
174+
..AutoFaker.Faker.Random.ListItems(before.Components)
175+
.Select(createdCfc =>
176+
{
177+
var copy = createdCfc.Copy();
178+
copy.ComplexFormHeadword = after.Headword();
179+
return copy;
180+
}),
181+
// keep new
182+
..after.Components
183+
]),
184+
];
185+
186+
// expected should not be round-tripped, because an api might manipulate it somehow.
187+
// We expect the final result to be equivalent to this "raw"/untouched, requested state.
188+
var expected = after.Copy();
189+
190+
if (roundTripApi is not null)
191+
{
192+
// round-tripping ensures we're dealing with realistic data
193+
// (e.g. in fwdata ComplexFormComponents do not have an Id)
194+
before = await roundTripApi.CreateEntry(before);
195+
await roundTripApi.DeleteEntry(before.Id);
196+
after = await roundTripApi.CreateEntry(after);
197+
await roundTripApi.DeleteEntry(after.Id);
198+
}
199+
200+
// before should not be round-tripped here. That's handled above.
201+
await Api.CreateEntry(before);
202+
203+
// act
204+
await EntrySync.SyncFull(before, after, Api);
205+
var actual = await Api.GetEntry(after.Id);
206+
207+
// assert
208+
actual.Should().NotBeNull();
209+
actual.Should().BeEquivalentTo(after, options =>
210+
{
211+
options = options
212+
.WithStrictOrdering()
213+
.WithoutStrictOrderingFor(e => e.ComplexForms) // sorted alphabetically
214+
.WithoutStrictOrderingFor(e => e.Path.EndsWith($".{nameof(Sense.SemanticDomains)}")) // not sorted
215+
.For(e => e.Senses).Exclude(s => s.Order)
216+
.For(e => e.Components).Exclude(c => c.Order)
217+
.For(e => e.ComplexForms).Exclude(c => c.Order)
218+
.For(e => e.Senses).For(s => s.ExampleSentences).Exclude(e => e.Order);
219+
if (currentApiType == ApiType.Crdt)
220+
{
221+
// does not yet update Headwords 😕
222+
options = options
223+
.For(e => e.Components).Exclude(c => c.ComplexFormHeadword)
224+
.For(e => e.ComplexForms).Exclude(c => c.ComponentHeadword);
225+
}
226+
if (currentApiType == ApiType.FwData)
227+
{
228+
// does not support changing MorphType yet (see UpdateEntryProxy.MorphType)
229+
options = options.Excluding(e => e.MorphType);
230+
}
231+
return options;
232+
});
233+
}
234+
70235
[Fact]
71236
public async Task NormalizesStringsToNFD()
72237
{

0 commit comments

Comments
 (0)