From c3beeccc8783c5013465440abe481d8d0836e5d9 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Tue, 29 Sep 2026 17:08:57 -0400 Subject: [PATCH 1/4] LT-22725: Cover natural class abbreviation display with tests A rule formula draws a feature-based natural class the user has named as its abbreviation in brackets, and any other one as its feature list. Tests now pin that choice, the line count and cell width that follow from it, and the display dependencies that rebuild a formula when a name or abbreviation is edited. A collector environment drives the view constructors with no window. It captures the drawn text and the dependencies registered, and it measures one unit per character, so a context's cell width can be asserted against the text that context draws. A registration naming more pairs than it supplies is rejected rather than trimmed, so the harness cannot absorb a count that disagrees with its arrays. Two cases are pinned as current behavior rather than as a fix. A class abbreviated "C" shows its features unless it also carries a name, so the abbreviation alone does not decide the drawing. A segment-based class is sized to its abbreviation while registration covers feature-based classes, so editing that abbreviation leaves the cell at its earlier width. Co-Authored-By: Claude Opus 5 --- ...RuleFormulaControlNaturalClassNameTests.cs | 101 +++++++ ...uleFormulaVcNaturalClassDependencyTests.cs | 221 +++++++++++++++ .../RuleFormulaVcNaturalClassDisplayTests.cs | 138 ++++++++++ .../RuleFormulaVcNaturalClassSizingTests.cs | 134 ++++++++++ .../RuleFormulaVcTestBase.cs | 252 ++++++++++++++++++ .../RuleFormulaVcTestDoubles.cs | 162 +++++++++++ 6 files changed, 1008 insertions(+) create mode 100644 Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs create mode 100644 Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs create mode 100644 Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDisplayTests.cs create mode 100644 Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassSizingTests.cs create mode 100644 Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs create mode 100644 Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestDoubles.cs diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs new file mode 100644 index 0000000000..a0af6e2ec5 --- /dev/null +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs @@ -0,0 +1,101 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using NUnit.Framework; +using SIL.LCModel; +using FeatVals = System.Collections.Generic.Dictionary; + +namespace SIL.FieldWorks.XWorks.MorphologyEditor +{ + /// + /// Which feature-based natural classes count as carrying a name the user gave them. This is + /// the single input deciding whether a rule formula draws a class as an abbreviation or as a + /// feature list, so each shape of name a project can hold is pinned here. + /// + [TestFixture] + public class RuleFormulaControlNaturalClassNameTests : RuleFormulaVcTestBase + { + [Test] + public void NoClass_IsNotUserDefined() + { + Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(null), Is.False); + } + + [Test] + public void ClassWithNoName_IsNotUserDefined() + { + IPhNCFeatures natClass = AddFeatureNaturalClass(null, "Vd", + new FeatVals { { "vd", "+" } }); + + Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.False, + "an unnamed class reads back as a placeholder rather than as empty text"); + } + + [Test] + public void ClassWithAnEmptyName_IsNotUserDefined() + { + IPhNCFeatures natClass = AddFeatureNaturalClass(string.Empty, "Vd", + new FeatVals { { "vd", "+" } }); + + Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.False); + } + + [Test] + public void ClassNamedForARule_IsNotUserDefined() + { + IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", + new FeatVals { { "vd", "+" } }); + + Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.False); + } + + /// + /// Projects hold rule-generated names that carry a prefix ahead of the generated text, so + /// the generated text is recognised wherever it sits in the name. + /// + [Test] + public void ClassNamedForARuleBehindAPrefix_IsNotUserDefined() + { + IPhNCFeatures natClass = AddFeatureNaturalClass(null, null, + new FeatVals { { "vd", "+" } }); + natClass.Name.SetUserWritingSystem("Phonemes s and t - " + + string.Format(MEStrings.ksRuleNCFeatsName, "s to n")); + + Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.False); + } + + [Test] + public void NamedClass_IsUserDefined() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + + Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.True); + } + + /// + /// The natural class editor writes a name into the analysis writing systems while the + /// check reads the user writing system, so the name has to be found across writing + /// systems for a named class to qualify. + /// + [Test] + public void ClassNamedInTheVernacularWritingSystemOnly_IsUserDefined() + { + IPhNCFeatures natClass = AddFeatureNaturalClass(null, "Vd", + new FeatVals { { "vd", "+" } }); + natClass.Name.SetVernacularDefaultWritingSystem("Voiced"); + + Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.True); + } + + [Test] + public void NamedClassWithNoFeatures_IsUserDefined() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", new FeatVals()); + + Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.True, + "a name qualifies a class whatever it carries for features"); + } + } +} diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs new file mode 100644 index 0000000000..8f04c503bd --- /dev/null +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs @@ -0,0 +1,221 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Linq; +using NUnit.Framework; +using SIL.LCModel; +using FeatVals = System.Collections.Generic.Dictionary; + +namespace SIL.FieldWorks.XWorks.MorphologyEditor +{ + /// + /// The display dependencies a rule formula registers on the natural classes it draws. A class + /// name decides whether the formula draws an abbreviation or a feature list, and both the + /// name + /// and the abbreviation decide the cell size, so editing either has to rebuild the formula. + /// + [TestFixture] + public class RuleFormulaVcNaturalClassDependencyTests : RuleFormulaVcTestBase + { + [Test] + public void RegularRule_RegistersTheNameAndAbbreviationOfItsClass() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + AddStrucDescNCContext(rhs, natClass); + + RecordingCollectorEnv env = DrawRegularRule(rhs); + + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidName), Is.True); + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidAbbreviation), Is.True); + } + + [Test] + public void RegularRule_RegistersAClassInItsStructuralChange() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Nasal", "N", + new FeatVals { { "nasal", "+" } }); + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + AddStrucChangeNCContext(rhs, natClass); + + RecordingCollectorEnv env = DrawRegularRule(rhs); + + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidName), Is.True); + } + + [Test] + public void RegularRule_RegistersAClassNestedInASequenceContext() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + rhs.LeftContextOA = AddSequenceContext(AddStandaloneNCContext(natClass)); + + RecordingCollectorEnv env = DrawRegularRule(rhs); + + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidName), Is.True); + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidAbbreviation), Is.True); + } + + [Test] + public void RegularRule_RegistersAClassInsideAnIterationContext() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + rhs.RightContextOA = AddIterationContext(AddStandaloneNCContext(natClass)); + + RecordingCollectorEnv env = DrawRegularRule(rhs); + + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidName), Is.True); + } + + [Test] + public void RegularRule_WithAnEmptyIterationContext_DrawsWithoutError() + { + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + rhs.LeftContextOA = AddIterationContext(null); + + Assert.That(() => DrawRegularRule(rhs), Throws.Nothing); + } + + [Test] + public void RegularRule_WithoutSurroundingContexts_DrawsWithoutError() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + AddStrucDescNCContext(rhs, natClass); + + Assert.That(rhs.LeftContextOA, Is.Null); + Assert.That(rhs.RightContextOA, Is.Null); + Assert.That(() => DrawRegularRule(rhs), Throws.Nothing); + } + + [Test] + public void RegularRule_WithoutFeatureClasses_RegistersNoNaturalClassDependency() + { + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + + RecordingCollectorEnv env = DrawRegularRule(rhs); + + Assert.That(NamesRegistered(env), Is.Empty); + } + + [Test] + public void RegularRule_RegistersEveryClassItDraws() + { + IPhNCFeatures first = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + IPhNCFeatures second = AddFeatureNaturalClass("Nasal", "N", + new FeatVals { { "nasal", "+" } }); + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + AddStrucDescNCContext(rhs, first); + AddStrucChangeNCContext(rhs, second); + + RecordingCollectorEnv env = DrawRegularRule(rhs); + + Assert.That(NamesRegistered(env), Is.EquivalentTo(new[] { first.Hvo, second.Hvo })); + } + + [Test] + public void AffixProcessRule_RegistersTheClassInItsInput() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + IMoAffixProcess rule = AddAffixProcessRule(); + AddInputNCContext(rule, natClass); + + var vc = new AffixRuleFormulaVc(Cache, m_propertyTable); + var env = new RecordingCollectorEnv(Cache.MainCacheAccessor, rule.Hvo); + vc.Display(env, rule.Hvo, AffixRuleFormulaVc.kfragRule); + + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidName), Is.True); + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidAbbreviation), Is.True); + } + + [Test] + public void MetathesisRule_RegistersTheNameAndAbbreviationOfItsClass() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + IPhMetathesisRule rule = AddMetathesisRule("t s to s t"); + AddStrucDescNCContext(rule, natClass); + + RecordingCollectorEnv env = DrawMetathesisRule(rule); + + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidName), Is.True); + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidAbbreviation), Is.True); + } + + [Test] + public void MetathesisRule_RegistersEveryClassItDraws() + { + IPhNCFeatures first = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + IPhNCFeatures second = AddFeatureNaturalClass("Nasal", "N", + new FeatVals { { "nasal", "+" } }); + IPhMetathesisRule rule = AddMetathesisRule("t s to s t"); + AddStrucDescNCContext(rule, first); + AddStrucDescNCContext(rule, second); + + RecordingCollectorEnv env = DrawMetathesisRule(rule); + + Assert.That(NamesRegistered(env), Is.EquivalentTo(new[] { first.Hvo, second.Hvo })); + } + + [Test] + public void MetathesisRule_WithoutContexts_DrawsWithoutError() + { + IPhMetathesisRule rule = AddMetathesisRule("t s to s t"); + + Assert.That(() => DrawMetathesisRule(rule), Throws.Nothing); + } + + /// + /// A segment-based class is drawn as its abbreviation and its cell is sized to it, while + /// registration covers feature-based classes. Editing a segment class's abbreviation + /// therefore leaves the cell at the width the old abbreviation asked for until something + /// else rebuilds the formula. + /// + [Test] + public void RegularRule_DoesNotRegisterASegmentClass() + { + IPhNCSegments natClass = AddSegmentNaturalClass("Consonant", "C"); + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + AddStrucDescNCContext(rhs, natClass); + + RecordingCollectorEnv env = DrawRegularRule(rhs); + + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidAbbreviation), Is.False); + } + + private RecordingCollectorEnv DrawRegularRule(IPhSegRuleRHS rhs) + { + var vc = new RegRuleFormulaVc(Cache, m_propertyTable); + var env = new RecordingCollectorEnv(Cache.MainCacheAccessor, rhs.Hvo); + vc.Display(env, rhs.Hvo, RegRuleFormulaVc.kfragRHS); + return env; + } + + private RecordingCollectorEnv DrawMetathesisRule(IPhMetathesisRule rule) + { + var vc = new MetaRuleFormulaVc(Cache, m_propertyTable); + var env = new RecordingCollectorEnv(Cache.MainCacheAccessor, rule.Hvo); + vc.Display(env, rule.Hvo, MetaRuleFormulaVc.kfragRule); + return env; + } + + private static int[] NamesRegistered(RecordingCollectorEnv env) + { + return env.Dependencies + .SelectMany(call => Enumerable.Range(0, call.Count) + .Where(i => call.Tags[i] == PhNaturalClassTags.kflidName) + .Select(i => call.Hvos[i])) + .Distinct() + .ToArray(); + } + } +} diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDisplayTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDisplayTests.cs new file mode 100644 index 0000000000..0f43f54756 --- /dev/null +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDisplayTests.cs @@ -0,0 +1,138 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using NUnit.Framework; +using SIL.LCModel; +using FeatVals = System.Collections.Generic.Dictionary; + +namespace SIL.FieldWorks.XWorks.MorphologyEditor +{ + /// + /// What a rule formula draws for a natural class context: a class the user has named shows + /// its + /// abbreviation in brackets, and any other feature-based class shows its feature list. + /// + [TestFixture] + public class RuleFormulaVcNaturalClassDisplayTests : RuleFormulaVcTestBase + { + private const string LeftBracketUpHook = "\u23a1"; + private const string LeftBracketLowHook = "\u23a3"; + + [Test] + public void NamedClass_WithOneFeature_DrawsAbbreviation() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + + Assert.That(DrawContext(AddStandaloneNCContext(natClass)), Is.EqualTo("[Vd]")); + } + + [Test] + public void NamedClass_WithThreeFeatures_DrawsAbbreviationOnly() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced stop", "Vd", + new FeatVals { { "vd", "+" }, { "cons", "+" }, { "cont", "-" } }); + + Assert.That(DrawContext(AddStandaloneNCContext(natClass)), Is.EqualTo("[Vd]"), + "a named class is drawn as its abbreviation however many features it carries"); + } + + [Test] + public void NamedClass_WithNoAbbreviation_DrawsStars() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", null, + new FeatVals { { "vd", "+" } }); + + Assert.That(DrawContext(AddStandaloneNCContext(natClass)), Is.EqualTo("[***]")); + } + + [Test] + public void NamedClass_WithEmptyAbbreviation_DrawsStars() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", string.Empty, + new FeatVals { { "vd", "+" } }); + + Assert.That(DrawContext(AddStandaloneNCContext(natClass)), Is.EqualTo("[***]")); + } + + [Test] + public void RuleNamedClass_WithOneFeature_DrawsFeature() + { + IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", + new FeatVals { { "vd", "+" } }); + + Assert.That(DrawContext(AddStandaloneNCContext(natClass)), Is.EqualTo("[+ vd]")); + } + + [Test] + public void RuleNamedClass_WithThreeFeatures_DrawsEveryFeatureInAPile() + { + IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", + new FeatVals { { "vd", "+" }, { "cons", "+" }, { "cont", "-" } }); + + string drawn = DrawContext(AddStandaloneNCContext(natClass)); + + Assert.That(drawn, Does.Contain("+ vd")); + Assert.That(drawn, Does.Contain("+ cons")); + Assert.That(drawn, Does.Contain("- cont")); + Assert.That(drawn, Does.Contain(LeftBracketUpHook), + "a class drawn over several lines is bracketed by hooks, not square brackets"); + Assert.That(drawn, Does.Contain(LeftBracketLowHook)); + } + + [Test] + public void RuleNamedClass_WithNoFeatures_DrawsQuestions() + { + IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", new FeatVals()); + + Assert.That(DrawContext(AddStandaloneNCContext(natClass)), Is.EqualTo("[???]")); + } + + /// + /// Pins which of the two inputs decides the drawing for a class abbreviated "C": the + /// abbreviation alone does not qualify a class, so a rule-named one shows its features. + /// + [Test] + public void RuleNamedClass_AbbreviatedC_DrawsFeature() + { + IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", + new FeatVals { { "cons", "+" } }); + natClass.Abbreviation.SetAnalysisDefaultWritingSystem("C"); + + Assert.That(DrawContext(AddStandaloneNCContext(natClass)), Is.EqualTo("[+ cons]")); + } + + [Test] + public void SegmentClass_DrawsAbbreviation() + { + IPhNCSegments natClass = AddSegmentNaturalClass("Consonant", "C"); + + Assert.That(DrawContext(AddStandaloneNCContext(natClass)), Is.EqualTo("[C]")); + } + + [Test] + public void NamedClass_InsideAnIterationContext_DrawsAbbreviation() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + IPhSimpleContextNC member = AddStandaloneNCContext(natClass); + + Assert.That(DrawContext(AddIterationContext(member)), Does.Contain("[Vd]")); + } + + [Test] + public void ClassWithNoFeatureStructure_DrawsQuestions() + { + Assert.That(DrawContext(AddStandaloneNCContext(null)), Is.EqualTo("[???]")); + } + + private string DrawContext(IPhContextOrVar ctxtOrVar) + { + var vc = new TestRuleFormulaVc(Cache, m_propertyTable); + var env = new RecordingCollectorEnv(Cache.MainCacheAccessor, ctxtOrVar.Hvo); + vc.Display(env, ctxtOrVar.Hvo, RuleFormulaVcBase.kfragContext); + return env.Text; + } + } +} diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassSizingTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassSizingTests.cs new file mode 100644 index 0000000000..62598c829e --- /dev/null +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassSizingTests.cs @@ -0,0 +1,134 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Linq; +using NUnit.Framework; +using SIL.LCModel; +using FeatVals = System.Collections.Generic.Dictionary; + +namespace SIL.FieldWorks.XWorks.MorphologyEditor +{ + /// + /// The line count and cell width a rule formula reserves for a natural class context. A cell + /// sized from anything other than what the context draws leaves the drawing clipped or the + /// cell padded, so the governing assertion is that the two agree. + /// + [TestFixture] + public class RuleFormulaVcNaturalClassSizingTests : RuleFormulaVcTestBase + { + [Test] + public void NamedClass_WithOneFeature_IsSizedToItsAbbreviation() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", + new FeatVals { { "vd", "+" } }); + + AssertCellMatchesDrawing(AddStandaloneNCContext(natClass), "[Vd]"); + } + + [Test] + public void NamedClass_WithThreeFeatures_IsSizedToItsAbbreviation() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced stop", "Vd", + new FeatVals { { "vd", "+" }, { "cons", "+" }, { "cont", "-" } }); + + AssertCellMatchesDrawing(AddStandaloneNCContext(natClass), "[Vd]"); + } + + [Test] + public void NamedClass_WithNoAbbreviation_IsSizedToTheStars() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", null, + new FeatVals { { "vd", "+" } }); + + AssertCellMatchesDrawing(AddStandaloneNCContext(natClass), "[***]"); + } + + [Test] + public void RuleNamedClass_WithOneFeature_IsSizedToItsFeature() + { + IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", + new FeatVals { { "vd", "+" } }); + + AssertCellMatchesDrawing(AddStandaloneNCContext(natClass), "[+ vd]"); + } + + [Test] + public void NamedClass_WithLongAbbreviation_IsSizedToTheWholeAbbreviation() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiceless aspirated stop", + "VlAspStop", new FeatVals { { "vd", "-" }, { "cons", "+" } }); + + AssertCellMatchesDrawing(AddStandaloneNCContext(natClass), "[VlAspStop]"); + } + + [Test] + public void NamedClass_WithThreeFeatures_OccupiesOneLine() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced stop", "Vd", + new FeatVals { { "vd", "+" }, { "cons", "+" }, { "cont", "-" } }); + + Assert.That(NumLinesFor(AddStandaloneNCContext(natClass)), Is.EqualTo(1)); + } + + [Test] + public void RuleNamedClass_WithThreeFeatures_OccupiesOneLinePerFeature() + { + IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", + new FeatVals { { "vd", "+" }, { "cons", "+" }, { "cont", "-" } }); + + Assert.That(NumLinesFor(AddStandaloneNCContext(natClass)), Is.EqualTo(3)); + } + + [Test] + public void RuleNamedClass_WithNoFeatures_OccupiesNoLines() + { + IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", new FeatVals()); + + Assert.That(NumLinesFor(AddStandaloneNCContext(natClass)), Is.EqualTo(0)); + } + + /// + /// A formula is as tall as its tallest context, so naming the only many-featured class in + /// a rule brings the whole formula down to a single line. + /// + [Test] + public void NamedClass_IsTheHeightOfAFormulaThatHoldsNothingTaller() + { + IPhNCFeatures named = AddFeatureNaturalClass("Voiced stop", "Vd", + new FeatVals { { "vd", "+" }, { "cons", "+" }, { "cont", "-" } }); + IPhNCSegments segments = AddSegmentNaturalClass("Consonant", "C"); + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + AddStrucDescNCContext(rhs, named); + AddStrucChangeNCContext(rhs, segments); + + var vc = new TestRuleFormulaVc(Cache, m_propertyTable); + int tallest = rhs.OwningRule.StrucDescOS.Concat(rhs.StrucChangeOS) + .Max(ctxt => vc.NumLinesFor(ctxt)); + + Assert.That(tallest, Is.EqualTo(1)); + } + + private int NumLinesFor(IPhContextOrVar ctxtOrVar) + { + return new TestRuleFormulaVc(Cache, m_propertyTable).NumLinesFor(ctxtOrVar); + } + + /// + /// Asserts that the context draws the expected text and that its cell is sized to exactly + /// that text plus the margins every context carries. String measurement in this + /// environment reports one unit per character, so the two are directly comparable. + /// + private void AssertCellMatchesDrawing(IPhContextOrVar ctxtOrVar, string expected) + { + var vc = new TestRuleFormulaVc(Cache, m_propertyTable); + var env = new RecordingCollectorEnv(Cache.MainCacheAccessor, ctxtOrVar.Hvo); + vc.Display(env, ctxtOrVar.Hvo, RuleFormulaVcBase.kfragContext); + + Assert.That(env.Text, Is.EqualTo(expected)); + Assert.That(vc.WidthOf(ctxtOrVar, env), + Is.EqualTo(expected.Length + vc.ContextMargins), + "the cell is sized to the text the context draws"); + } + } +} diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs new file mode 100644 index 0000000000..d199ac2162 --- /dev/null +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs @@ -0,0 +1,252 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System.Collections.Generic; +using System.Linq; +using SIL.LCModel; +using XCore; +using FeatVals = System.Collections.Generic.Dictionary; + +namespace SIL.FieldWorks.XWorks.MorphologyEditor +{ + /// + /// Fixture support for the rule formula view constructors: an in-memory phonological feature + /// system, builders for the natural classes and rule contexts a formula draws, and a property + /// table the view constructors can measure fonts against. + /// + public abstract class RuleFormulaVcTestBase : MemoryOnlyBackendProviderRestoredForEachTestTestBase + { + private Mediator m_mediator; + + /// + /// The property table the view constructors under test are constructed with. It + /// carries no stylesheet, so font measurement falls back to writing system defaults. + /// + protected PropertyTable m_propertyTable; + + public override void TestSetup() + { + base.TestSetup(); + m_mediator = new Mediator(); + m_propertyTable = new PropertyTable(m_mediator); + } + + public override void TestTearDown() + { + m_propertyTable?.Dispose(); + m_mediator?.Dispose(); + m_propertyTable = null; + m_mediator = null; + base.TestTearDown(); + } + + /// + /// Adds the closed phonological features the natural class builders draw values from. + /// + protected override void CreateTestData() + { + base.CreateTestData(); + AddClosedFeature("vd", "+", "-"); + AddClosedFeature("cons", "+", "-"); + AddClosedFeature("cont", "+", "-"); + AddClosedFeature("nasal", "+", "-"); + } + + /// + /// Adds a closed phonological feature carrying the given symbolic values, naming the + /// feature and each of its values after its own abbreviation. + /// + protected IFsClosedFeature AddClosedFeature(string abbreviation, params string[] values) + { + IFsClosedFeature feature = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.PhFeatureSystemOA.FeaturesOC.Add(feature); + feature.Name.SetAnalysisDefaultWritingSystem(abbreviation); + feature.Abbreviation.SetAnalysisDefaultWritingSystem(abbreviation); + foreach (string value in values) + { + IFsSymFeatVal symbol = Cache.ServiceLocator.GetInstance().Create(); + feature.ValuesOC.Add(symbol); + symbol.Name.SetAnalysisDefaultWritingSystem(value); + symbol.Abbreviation.SetAnalysisDefaultWritingSystem(value); + } + return feature; + } + + /// + /// Adds a feature-based natural class carrying the given feature values. The name and + /// abbreviation are set in the default analysis writing system, which is where the + /// natural + /// class editor puts them; either may be null to leave it unset. + /// + protected IPhNCFeatures AddFeatureNaturalClass(string name, string abbreviation, FeatVals featVals) + { + IPhNCFeatures natClass = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.PhonologicalDataOA.NaturalClassesOS.Add(natClass); + if (name != null) + natClass.Name.SetAnalysisDefaultWritingSystem(name); + if (abbreviation != null) + natClass.Abbreviation.SetAnalysisDefaultWritingSystem(abbreviation); + natClass.FeaturesOA = Cache.ServiceLocator.GetInstance().Create(); + foreach (KeyValuePair featVal in featVals) + AddClosedValue(natClass.FeaturesOA, featVal.Key, featVal.Value); + return natClass; + } + + /// + /// Adds a feature-based natural class named the way one built for a rule is named, in the + /// user writing system and with no abbreviation. + /// + protected IPhNCFeatures AddRuleNamedFeatureNaturalClass(string ruleName, FeatVals featVals) + { + IPhNCFeatures natClass = AddFeatureNaturalClass(null, null, featVals); + natClass.Name.SetUserWritingSystem(string.Format(MEStrings.ksRuleNCFeatsName, ruleName)); + return natClass; + } + + /// + /// Adds a segment-based natural class with the given name and abbreviation. + /// + protected IPhNCSegments AddSegmentNaturalClass(string name, string abbreviation) + { + IPhNCSegments natClass = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.PhonologicalDataOA.NaturalClassesOS.Add(natClass); + natClass.Name.SetAnalysisDefaultWritingSystem(name); + natClass.Abbreviation.SetAnalysisDefaultWritingSystem(abbreviation); + return natClass; + } + + /// + /// Adds a regular phonological rule and returns its right-hand side. + /// + protected IPhSegRuleRHS AddRegularRule(string name) + { + IPhRegularRule rule = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.PhonologicalDataOA.PhonRulesOS.Add(rule); + rule.Name.SetAnalysisDefaultWritingSystem(name); + return rule.RightHandSidesOS[0]; + } + + /// + /// Adds a metathesis rule. Its structural change indices are left unset, so it holds + /// contexts without assigning any of them to a switch or environment cell. + /// + protected IPhMetathesisRule AddMetathesisRule(string name) + { + IPhMetathesisRule rule = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.PhonologicalDataOA.PhonRulesOS.Add(rule); + rule.Name.SetAnalysisDefaultWritingSystem(name); + return rule; + } + + /// + /// Adds a natural class simple context to a metathesis rule's structural description, + /// assigning the class once the context is owned. + /// + protected IPhSimpleContextNC AddStrucDescNCContext(IPhMetathesisRule rule, IPhNaturalClass natClass) + { + IPhSimpleContextNC ctxt = Cache.ServiceLocator.GetInstance().Create(); + rule.StrucDescOS.Add(ctxt); + ctxt.FeatureStructureRA = natClass; + return ctxt; + } + + /// + /// Adds an affix process rule as the lexeme form of a new entry, with an empty input + /// sequence. + /// + protected IMoAffixProcess AddAffixProcessRule() + { + ILexEntry entry = Cache.ServiceLocator.GetInstance().Create(); + IMoAffixProcess rule = Cache.ServiceLocator.GetInstance().Create(); + entry.LexemeFormOA = rule; + rule.InputOS.Clear(); + return rule; + } + + /// + /// Adds a natural class simple context to an affix process rule's input, assigning the + /// class once the context is owned. + /// + protected IPhSimpleContextNC AddInputNCContext(IMoAffixProcess rule, IPhNaturalClass natClass) + { + IPhSimpleContextNC ctxt = Cache.ServiceLocator.GetInstance().Create(); + rule.InputOS.Add(ctxt); + ctxt.FeatureStructureRA = natClass; + return ctxt; + } + + /// + /// Adds a natural class simple context to a rule's structural description, assigning the + /// class once the context is owned. + /// + protected IPhSimpleContextNC AddStrucDescNCContext(IPhSegRuleRHS rhs, IPhNaturalClass natClass) + { + IPhSimpleContextNC ctxt = Cache.ServiceLocator.GetInstance().Create(); + rhs.OwningRule.StrucDescOS.Add(ctxt); + ctxt.FeatureStructureRA = natClass; + return ctxt; + } + + /// + /// Adds a natural class simple context to a rule's structural change, assigning the class + /// once the context is owned. + /// + protected IPhSimpleContextNC AddStrucChangeNCContext(IPhSegRuleRHS rhs, IPhNaturalClass natClass) + { + IPhSimpleContextNC ctxt = Cache.ServiceLocator.GetInstance().Create(); + rhs.StrucChangeOS.Add(ctxt); + ctxt.FeatureStructureRA = natClass; + return ctxt; + } + + /// + /// Adds a natural class simple context owned by the phonological data, so it can be a + /// member of a sequence or iteration context. + /// + protected IPhSimpleContextNC AddStandaloneNCContext(IPhNaturalClass natClass) + { + IPhSimpleContextNC ctxt = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.PhonologicalDataOA.ContextsOS.Add(ctxt); + ctxt.FeatureStructureRA = natClass; + return ctxt; + } + + /// + /// Adds a sequence context over the given members, which must already be owned. + /// + protected IPhSequenceContext AddSequenceContext(params IPhPhonContext[] members) + { + IPhSequenceContext ctxt = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.PhonologicalDataOA.ContextsOS.Add(ctxt); + foreach (IPhPhonContext member in members) + ctxt.MembersRS.Add(member); + return ctxt; + } + + /// + /// Adds an iteration context over the given member, which may be null. + /// + protected IPhIterationContext AddIterationContext(IPhPhonContext member) + { + IPhIterationContext ctxt = Cache.ServiceLocator.GetInstance().Create(); + Cache.LanguageProject.PhonologicalDataOA.ContextsOS.Add(ctxt); + ctxt.MemberRA = member; + ctxt.Minimum = 1; + ctxt.Maximum = -1; + return ctxt; + } + + private void AddClosedValue(IFsFeatStruc featStruc, string featureAbbr, string valueAbbr) + { + IFsClosedFeature feature = (IFsClosedFeature) Cache.LanguageProject.PhFeatureSystemOA + .FeaturesOC.First(f => f.Abbreviation.AnalysisDefaultWritingSystem.Text == featureAbbr); + IFsSymFeatVal symbol = feature.ValuesOC + .First(v => v.Abbreviation.AnalysisDefaultWritingSystem.Text == valueAbbr); + IFsClosedValue closedValue = Cache.ServiceLocator.GetInstance().Create(); + featStruc.FeatureSpecsOC.Add(closedValue); + closedValue.FeatureRA = feature; + closedValue.ValueRA = symbol; + } + } +} diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestDoubles.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestDoubles.cs new file mode 100644 index 0000000000..53d06c1ff2 --- /dev/null +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestDoubles.cs @@ -0,0 +1,162 @@ +// Copyright (c) 2026 SIL International +// This software is licensed under the LGPL, version 2.1 or later +// (http://www.gnu.org/licenses/lgpl-2.1.html) + +using System; +using System.Collections.Generic; +using System.Linq; +using SIL.FieldWorks.Common.RootSites; +using SIL.FieldWorks.Common.ViewsInterfaces; +using SIL.LCModel; +using SIL.LCModel.Core.KernelInterfaces; +using XCore; + +namespace SIL.FieldWorks.XWorks.MorphologyEditor +{ + /// + /// A concrete rule formula view constructor that draws a single context, so the shared + /// context-drawing, line-counting, and cell-sizing behavior can be exercised on its own. + /// + internal sealed class TestRuleFormulaVc : RuleFormulaVcBase + { + internal TestRuleFormulaVc(LcmCache cache, PropertyTable propertyTable) + : base(cache, propertyTable) + { + MaxNumLines = 1; + } + + /// + /// The line count the drawing code pads single-line piles out to. + /// + internal int MaxNumLines { get; set; } + + /// + /// The number of lines the given context occupies. + /// + internal int NumLinesFor(IPhContextOrVar ctxtOrVar) + { + return GetNumLines(ctxtOrVar); + } + + /// + /// The cell width the given context is sized to, in the units this environment's + /// string measurement reports. + /// + internal int WidthOf(IPhContextOrVar ctxtOrVar, IVwEnv vwenv) + { + return GetWidth(ctxtOrVar, vwenv); + } + + /// + /// The margin a context's cell carries either side of its drawing. + /// + internal int ContextMargins + { + get { return PileMargin * 2; } + } + + protected override int GetMaxNumLines() + { + return MaxNumLines; + } + + protected override int GetVarIndex(IPhFeatureConstraint var) + { + return -1; + } + } + + /// + /// A collector environment that captures both the text a view constructor draws and the + /// display dependencies it registers. String measurement reports one unit per character, so + /// a drawn string and the width it is measured at are directly comparable. + /// + internal sealed class RecordingCollectorEnv : StringCollectorEnv + { + private const char ZeroWidthSpace = '\u200b'; + + private readonly List m_dependencies = new List(); + + internal RecordingCollectorEnv(ISilDataAccess sda, int hvoRoot) + : base(null, sda, hvoRoot) + { + } + + /// + /// The drawn text with the zero-width boundary markers of each pile removed. + /// + internal string Text + { + get { return Result.Replace(ZeroWidthSpace.ToString(), string.Empty); } + } + + /// + /// Every dependency registration, in the order it was made. + /// + internal IReadOnlyList Dependencies + { + get { return m_dependencies; } + } + + public override void NoteDependency(int[] rghvo, int[] rgtag, int chvo) + { + m_dependencies.Add(new DependencyCall(rghvo, rgtag, chvo)); + base.NoteDependency(rghvo, rgtag, chvo); + } + + /// + /// Reports whether any registration names the given object and property within + /// the count it declared. + /// + internal bool DependsOn(int hvo, int tag) + { + return m_dependencies.Any(call => call.Covers(hvo, tag)); + } + + /// + /// One call registering a batch of object and property pairs the display depends on. + /// + internal sealed class DependencyCall + { + /// + /// Initializes a record of one registration, rejecting a declared count that runs + /// past either array so that a registration whose count and pairs disagree fails + /// the test that provoked it. + /// + /// + /// The count is negative, or names more pairs than were supplied. + /// + internal DependencyCall(int[] hvos, int[] tags, int count) + { + if (count < 0 || count > hvos.Length || count > tags.Length) + { + throw new ArgumentException(string.Format( + "A dependency registration declared {0} pairs but supplied {1} objects" + + " and {2} properties.", count, hvos.Length, tags.Length)); + } + Hvos = hvos; + Tags = tags; + Count = count; + } + + /// The objects named, paired positionally with the properties. + internal int[] Hvos { get; } + + /// The properties named, paired positionally with the objects. + internal int[] Tags { get; } + + /// How many pairs the caller declared. + internal int Count { get; } + + internal bool Covers(int hvo, int tag) + { + for (int i = 0; i < Count; i++) + { + if (Hvos[i] == hvo && Tags[i] == tag) + return true; + } + return false; + } + } + } +} From e1f4082f7f6310d85abfebc564b1608c47879c4b Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Wed, 30 Sep 2026 11:15:12 -0400 Subject: [PATCH 2/4] LT-22725: Tie natural class display tests to the specified behavior The tests now follow the rule the natural class change was built to meet: a class the user created draws as its abbreviation, and a class generated for a rule keeps its feature list. Six tests are dropped because nothing in that change could make them fail. One exercised a branch the change never touched, one pinned an internal line count that contradicts what is drawn, one repeated a case the width check already covers, and three asserted either the test fixture or the absence of a registration. The formula-height test now draws the whole rule through the real view constructor instead of recomputing the height inside the test. The writing system test now covers a project whose analysis language differs from its interface language, which the natural class editor produces and a vernacular-only name does not. New tests cover naming a generated class, which switches it from its feature list to its abbreviation, the name dependency that switch relies on, and a named class that has no features yet. The segment-class test asserts the registration the formula should make and is ignored until that is fixed, instead of asserting the missing registration as correct. Co-Authored-By: Claude Opus 5.5 --- ...RuleFormulaControlNaturalClassNameTests.cs | 17 +++-- ...uleFormulaVcNaturalClassDependencyTests.cs | 70 ++++++++----------- .../RuleFormulaVcNaturalClassDisplayTests.cs | 41 ++++++++--- .../RuleFormulaVcNaturalClassSizingTests.cs | 46 +++++------- .../RuleFormulaVcTestBase.cs | 3 +- 5 files changed, 93 insertions(+), 84 deletions(-) diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs index a0af6e2ec5..d399aaae16 100644 --- a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs @@ -4,6 +4,8 @@ using NUnit.Framework; using SIL.LCModel; +using SIL.LCModel.Core.Text; +using SIL.LCModel.Core.WritingSystems; using FeatVals = System.Collections.Generic.Dictionary; namespace SIL.FieldWorks.XWorks.MorphologyEditor @@ -76,15 +78,22 @@ public void NamedClass_IsUserDefined() /// /// The natural class editor writes a name into the analysis writing systems while the - /// check reads the user writing system, so the name has to be found across writing - /// systems for a named class to qualify. + /// check reads the user writing system. A project whose analysis language differs from + /// its interface language must still have its named classes qualify. /// [Test] - public void ClassNamedInTheVernacularWritingSystemOnly_IsUserDefined() + public void ClassNamedInAnAnalysisWritingSystemOtherThanTheUserOne_IsUserDefined() { + Cache.ServiceLocator.WritingSystemManager.GetOrSet("de", + out CoreWritingSystemDefinition german); + Cache.LanguageProject.AddToCurrentAnalysisWritingSystems(german); IPhNCFeatures natClass = AddFeatureNaturalClass(null, "Vd", new FeatVals { { "vd", "+" } }); - natClass.Name.SetVernacularDefaultWritingSystem("Voiced"); + natClass.Name.set_String(german.Handle, + TsStringUtils.MakeString("Stimmhaft", german.Handle)); + Assert.That(natClass.Name.UserDefaultWritingSystem.Text, Is.EqualTo("Stimmhaft"), + "the name must reach the check through the analysis writing system, " + + "not through a placeholder"); Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.True); } diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs index 8f04c503bd..2324c60dfb 100644 --- a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs @@ -12,8 +12,9 @@ namespace SIL.FieldWorks.XWorks.MorphologyEditor /// /// The display dependencies a rule formula registers on the natural classes it draws. A class /// name decides whether the formula draws an abbreviation or a feature list, and both the - /// name - /// and the abbreviation decide the cell size, so editing either has to rebuild the formula. + /// name and the abbreviation decide the cell size, so editing either has to rebuild the + /// formula. The collector environment does not re-run layout, so these assert the + /// registration that triggers the rebuild rather than the rebuild itself. /// [TestFixture] public class RuleFormulaVcNaturalClassDependencyTests : RuleFormulaVcTestBase @@ -32,6 +33,23 @@ public void RegularRule_RegistersTheNameAndAbbreviationOfItsClass() Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidAbbreviation), Is.True); } + /// + /// Naming a class generated for a rule turns its feature list into an abbreviation, + /// so the formula has to depend on that class's name before the user names it. + /// + [Test] + public void RegularRule_RegistersTheNameOfARuleNamedClass() + { + IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", + new FeatVals { { "vd", "+" } }); + IPhSegRuleRHS rhs = AddRegularRule("s to n"); + AddStrucDescNCContext(rhs, natClass); + + RecordingCollectorEnv env = DrawRegularRule(rhs); + + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidName), Is.True); + } + [Test] public void RegularRule_RegistersAClassInItsStructuralChange() { @@ -72,6 +90,10 @@ public void RegularRule_RegistersAClassInsideAnIterationContext() Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidName), Is.True); } + /// + /// An iteration context loses its member when the class inside it is deleted, and the + /// formula must still draw. + /// [Test] public void RegularRule_WithAnEmptyIterationContext_DrawsWithoutError() { @@ -81,29 +103,6 @@ public void RegularRule_WithAnEmptyIterationContext_DrawsWithoutError() Assert.That(() => DrawRegularRule(rhs), Throws.Nothing); } - [Test] - public void RegularRule_WithoutSurroundingContexts_DrawsWithoutError() - { - IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", - new FeatVals { { "vd", "+" } }); - IPhSegRuleRHS rhs = AddRegularRule("s to n"); - AddStrucDescNCContext(rhs, natClass); - - Assert.That(rhs.LeftContextOA, Is.Null); - Assert.That(rhs.RightContextOA, Is.Null); - Assert.That(() => DrawRegularRule(rhs), Throws.Nothing); - } - - [Test] - public void RegularRule_WithoutFeatureClasses_RegistersNoNaturalClassDependency() - { - IPhSegRuleRHS rhs = AddRegularRule("s to n"); - - RecordingCollectorEnv env = DrawRegularRule(rhs); - - Assert.That(NamesRegistered(env), Is.Empty); - } - [Test] public void RegularRule_RegistersEveryClassItDraws() { @@ -166,22 +165,15 @@ public void MetathesisRule_RegistersEveryClassItDraws() Assert.That(NamesRegistered(env), Is.EquivalentTo(new[] { first.Hvo, second.Hvo })); } - [Test] - public void MetathesisRule_WithoutContexts_DrawsWithoutError() - { - IPhMetathesisRule rule = AddMetathesisRule("t s to s t"); - - Assert.That(() => DrawMetathesisRule(rule), Throws.Nothing); - } - /// - /// A segment-based class is drawn as its abbreviation and its cell is sized to it, while - /// registration covers feature-based classes. Editing a segment class's abbreviation - /// therefore leaves the cell at the width the old abbreviation asked for until something - /// else rebuilds the formula. + /// A segment-based class is drawn as its abbreviation and its cell is sized to it, so + /// editing that abbreviation has to rebuild the formula exactly as it does for a + /// feature-based class. /// [Test] - public void RegularRule_DoesNotRegisterASegmentClass() + [Ignore("LT-22725 follow-up: a segment-based natural class is sized to its abbreviation " + + "but the formula does not register a dependency on it.")] + public void RegularRule_RegistersTheAbbreviationOfASegmentClass() { IPhNCSegments natClass = AddSegmentNaturalClass("Consonant", "C"); IPhSegRuleRHS rhs = AddRegularRule("s to n"); @@ -189,7 +181,7 @@ public void RegularRule_DoesNotRegisterASegmentClass() RecordingCollectorEnv env = DrawRegularRule(rhs); - Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidAbbreviation), Is.False); + Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidAbbreviation), Is.True); } private RecordingCollectorEnv DrawRegularRule(IPhSegRuleRHS rhs) diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDisplayTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDisplayTests.cs index 0f43f54756..d4f6469427 100644 --- a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDisplayTests.cs +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDisplayTests.cs @@ -10,8 +10,7 @@ namespace SIL.FieldWorks.XWorks.MorphologyEditor { /// /// What a rule formula draws for a natural class context: a class the user has named shows - /// its - /// abbreviation in brackets, and any other feature-based class shows its feature list. + /// its abbreviation in brackets, and a class generated for a rule shows its feature list. /// [TestFixture] public class RuleFormulaVcNaturalClassDisplayTests : RuleFormulaVcTestBase @@ -38,6 +37,15 @@ public void NamedClass_WithThreeFeatures_DrawsAbbreviationOnly() "a named class is drawn as its abbreviation however many features it carries"); } + [Test] + public void NamedClass_WithNoFeatures_DrawsAbbreviation() + { + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", new FeatVals()); + + Assert.That(DrawContext(AddStandaloneNCContext(natClass)), Is.EqualTo("[Vd]"), + "a named class is drawn as its abbreviation even before it has features"); + } + [Test] public void NamedClass_WithNoAbbreviation_DrawsStars() { @@ -90,8 +98,9 @@ public void RuleNamedClass_WithNoFeatures_DrawsQuestions() } /// - /// Pins which of the two inputs decides the drawing for a class abbreviated "C": the - /// abbreviation alone does not qualify a class, so a rule-named one shows its features. + /// Whether a class was generated for a rule decides the drawing, not what its + /// abbreviation says: a generated class keeps its feature list even after someone + /// abbreviates it "C". /// [Test] public void RuleNamedClass_AbbreviatedC_DrawsFeature() @@ -103,6 +112,24 @@ public void RuleNamedClass_AbbreviatedC_DrawsFeature() Assert.That(DrawContext(AddStandaloneNCContext(natClass)), Is.EqualTo("[+ cons]")); } + /// + /// A class generated for a rule becomes the user's once they name it in the natural class + /// editor, so from then on the formula draws its abbreviation instead of its features. + /// + [Test] + public void RuleNamedClass_OnceNamed_DrawsAbbreviation() + { + IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", + new FeatVals { { "vd", "+" } }); + IPhSimpleContextNC ctxt = AddStandaloneNCContext(natClass); + Assert.That(DrawContext(ctxt), Is.EqualTo("[+ vd]"), "before the class is named"); + + natClass.Name.SetAnalysisDefaultWritingSystem("Voiced"); + natClass.Abbreviation.SetAnalysisDefaultWritingSystem("Vd"); + + Assert.That(DrawContext(ctxt), Is.EqualTo("[Vd]"), "after the class is named"); + } + [Test] public void SegmentClass_DrawsAbbreviation() { @@ -121,12 +148,6 @@ public void NamedClass_InsideAnIterationContext_DrawsAbbreviation() Assert.That(DrawContext(AddIterationContext(member)), Does.Contain("[Vd]")); } - [Test] - public void ClassWithNoFeatureStructure_DrawsQuestions() - { - Assert.That(DrawContext(AddStandaloneNCContext(null)), Is.EqualTo("[???]")); - } - private string DrawContext(IPhContextOrVar ctxtOrVar) { var vc = new TestRuleFormulaVc(Cache, m_propertyTable); diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassSizingTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassSizingTests.cs index 62598c829e..991e79e2e4 100644 --- a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassSizingTests.cs +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassSizingTests.cs @@ -2,7 +2,6 @@ // This software is licensed under the LGPL, version 2.1 or later // (http://www.gnu.org/licenses/lgpl-2.1.html) -using System.Linq; using NUnit.Framework; using SIL.LCModel; using FeatVals = System.Collections.Generic.Dictionary; @@ -17,6 +16,12 @@ namespace SIL.FieldWorks.XWorks.MorphologyEditor [TestFixture] public class RuleFormulaVcNaturalClassSizingTests : RuleFormulaVcTestBase { + /// + /// The glyphs that assemble a bracket spanning several lines. + /// + private static readonly char[] MultiLineBracketParts = + { '\u23a1', '\u23a2', '\u23a3', '\u23a4', '\u23a5', '\u23a6' }; + [Test] public void NamedClass_WithOneFeature_IsSizedToItsAbbreviation() { @@ -53,15 +58,6 @@ public void RuleNamedClass_WithOneFeature_IsSizedToItsFeature() AssertCellMatchesDrawing(AddStandaloneNCContext(natClass), "[+ vd]"); } - [Test] - public void NamedClass_WithLongAbbreviation_IsSizedToTheWholeAbbreviation() - { - IPhNCFeatures natClass = AddFeatureNaturalClass("Voiceless aspirated stop", - "VlAspStop", new FeatVals { { "vd", "-" }, { "cons", "+" } }); - - AssertCellMatchesDrawing(AddStandaloneNCContext(natClass), "[VlAspStop]"); - } - [Test] public void NamedClass_WithThreeFeatures_OccupiesOneLine() { @@ -80,33 +76,25 @@ public void RuleNamedClass_WithThreeFeatures_OccupiesOneLinePerFeature() Assert.That(NumLinesFor(AddStandaloneNCContext(natClass)), Is.EqualTo(3)); } - [Test] - public void RuleNamedClass_WithNoFeatures_OccupiesNoLines() - { - IPhNCFeatures natClass = AddRuleNamedFeatureNaturalClass("s to n", new FeatVals()); - - Assert.That(NumLinesFor(AddStandaloneNCContext(natClass)), Is.EqualTo(0)); - } - /// - /// A formula is as tall as its tallest context, so naming the only many-featured class in - /// a rule brings the whole formula down to a single line. + /// A formula is drawn as tall as its tallest context, so a named class with many features + /// must not stretch the rule holding it into a bracket pile several lines tall. /// [Test] - public void NamedClass_IsTheHeightOfAFormulaThatHoldsNothingTaller() + public void NamedClass_WithThreeFeatures_KeepsTheFormulaOnOneLine() { - IPhNCFeatures named = AddFeatureNaturalClass("Voiced stop", "Vd", + IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced stop", "Vd", new FeatVals { { "vd", "+" }, { "cons", "+" }, { "cont", "-" } }); - IPhNCSegments segments = AddSegmentNaturalClass("Consonant", "C"); IPhSegRuleRHS rhs = AddRegularRule("s to n"); - AddStrucDescNCContext(rhs, named); - AddStrucChangeNCContext(rhs, segments); + AddStrucDescNCContext(rhs, natClass); - var vc = new TestRuleFormulaVc(Cache, m_propertyTable); - int tallest = rhs.OwningRule.StrucDescOS.Concat(rhs.StrucChangeOS) - .Max(ctxt => vc.NumLinesFor(ctxt)); + var vc = new RegRuleFormulaVc(Cache, m_propertyTable); + var env = new RecordingCollectorEnv(Cache.MainCacheAccessor, rhs.Hvo); + vc.Display(env, rhs.Hvo, RegRuleFormulaVc.kfragRHS); - Assert.That(tallest, Is.EqualTo(1)); + Assert.That(env.Text, Does.Contain("[Vd]")); + Assert.That(env.Text.IndexOfAny(MultiLineBracketParts), Is.EqualTo(-1), + "no part of the formula is drawn with a bracket spanning several lines"); } private int NumLinesFor(IPhContextOrVar ctxtOrVar) diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs index d199ac2162..d09c7dc4a9 100644 --- a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs @@ -76,8 +76,7 @@ protected IFsClosedFeature AddClosedFeature(string abbreviation, params string[] /// /// Adds a feature-based natural class carrying the given feature values. The name and /// abbreviation are set in the default analysis writing system, which is where the - /// natural - /// class editor puts them; either may be null to leave it unset. + /// natural class editor puts them; either may be null to leave it unset. /// protected IPhNCFeatures AddFeatureNaturalClass(string name, string abbreviation, FeatVals featVals) { From 677f9acdfab5f9e7e11369359ed38bc9ca4aacd5 Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Wed, 30 Sep 2026 16:44:08 -0400 Subject: [PATCH 3/4] LT-22725: Leave natural class follow-up behavior to its own tickets The segment-class dependency test described behavior no change has delivered yet, so it sat ignored. That behavior, and the natural class naming gaps found while testing by hand, go into follow-up tickets whose fixes can add their tests as failing-first tests. The unnamed-class tests keep asserting that such a class carries no name the user gave it. That is the check's contract, and Set Phonological Features relies on it; their messages now say so instead of describing how an unset name reads back. Co-Authored-By: Claude Opus 5.5 --- ...RuleFormulaControlNaturalClassNameTests.cs | 10 ++++++++-- ...uleFormulaVcNaturalClassDependencyTests.cs | 19 ------------------- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs index d399aaae16..17e2139aa2 100644 --- a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaControlNaturalClassNameTests.cs @@ -24,6 +24,11 @@ public void NoClass_IsNotUserDefined() Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(null), Is.False); } + /// + /// The check answers whether a class carries a name the user gave it, and an unnamed + /// class carries none. Set Phonological Features relies on that answer to keep + /// offering feature edits on unnamed classes (LT-22576). + /// [Test] public void ClassWithNoName_IsNotUserDefined() { @@ -31,7 +36,7 @@ public void ClassWithNoName_IsNotUserDefined() new FeatVals { { "vd", "+" } }); Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.False, - "an unnamed class reads back as a placeholder rather than as empty text"); + "an unnamed class carries no name the user gave it"); } [Test] @@ -40,7 +45,8 @@ public void ClassWithAnEmptyName_IsNotUserDefined() IPhNCFeatures natClass = AddFeatureNaturalClass(string.Empty, "Vd", new FeatVals { { "vd", "+" } }); - Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.False); + Assert.That(RuleFormulaControl.IsFeatureBasedNCNameUserDefined(natClass), Is.False, + "an empty name is not a name the user gave the class"); } [Test] diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs index 2324c60dfb..387e24fa28 100644 --- a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs @@ -165,25 +165,6 @@ public void MetathesisRule_RegistersEveryClassItDraws() Assert.That(NamesRegistered(env), Is.EquivalentTo(new[] { first.Hvo, second.Hvo })); } - /// - /// A segment-based class is drawn as its abbreviation and its cell is sized to it, so - /// editing that abbreviation has to rebuild the formula exactly as it does for a - /// feature-based class. - /// - [Test] - [Ignore("LT-22725 follow-up: a segment-based natural class is sized to its abbreviation " - + "but the formula does not register a dependency on it.")] - public void RegularRule_RegistersTheAbbreviationOfASegmentClass() - { - IPhNCSegments natClass = AddSegmentNaturalClass("Consonant", "C"); - IPhSegRuleRHS rhs = AddRegularRule("s to n"); - AddStrucDescNCContext(rhs, natClass); - - RecordingCollectorEnv env = DrawRegularRule(rhs); - - Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidAbbreviation), Is.True); - } - private RecordingCollectorEnv DrawRegularRule(IPhSegRuleRHS rhs) { var vc = new RegRuleFormulaVc(Cache, m_propertyTable); From 53176c6b02b634d5bbb5404506cb69b674f5dd6f Mon Sep 17 00:00:00 2001 From: Zachary Burnham Date: Wed, 30 Sep 2026 17:18:20 -0400 Subject: [PATCH 4/4] LT-22725: Draw the metathesis classes the dependency tests check The metathesis dependency tests added classes to the rule's structural description without placing them in a switch cell, so the formula drew none of them and the tests checked registration for classes that never appear. The builder now places each class the way the formula editor does, and the tests assert that the class is drawn before checking what is registered. Co-Authored-By: Claude Opus 5.5 --- ...RuleFormulaVcNaturalClassDependencyTests.cs | 9 ++++++--- .../RuleFormulaVcTestBase.cs | 18 ++++++++++++------ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs index 387e24fa28..c8f2f59105 100644 --- a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcNaturalClassDependencyTests.cs @@ -141,10 +141,11 @@ public void MetathesisRule_RegistersTheNameAndAbbreviationOfItsClass() IPhNCFeatures natClass = AddFeatureNaturalClass("Voiced", "Vd", new FeatVals { { "vd", "+" } }); IPhMetathesisRule rule = AddMetathesisRule("t s to s t"); - AddStrucDescNCContext(rule, natClass); + AddMetathesisNCContext(rule, PhMetathesisRuleTags.kidxLeftSwitch, natClass); RecordingCollectorEnv env = DrawMetathesisRule(rule); + Assert.That(env.Text, Does.Contain("[Vd]"), "the class is drawn in its switch cell"); Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidName), Is.True); Assert.That(env.DependsOn(natClass.Hvo, PhNaturalClassTags.kflidAbbreviation), Is.True); } @@ -157,11 +158,13 @@ public void MetathesisRule_RegistersEveryClassItDraws() IPhNCFeatures second = AddFeatureNaturalClass("Nasal", "N", new FeatVals { { "nasal", "+" } }); IPhMetathesisRule rule = AddMetathesisRule("t s to s t"); - AddStrucDescNCContext(rule, first); - AddStrucDescNCContext(rule, second); + AddMetathesisNCContext(rule, PhMetathesisRuleTags.kidxLeftSwitch, first); + AddMetathesisNCContext(rule, PhMetathesisRuleTags.kidxRightSwitch, second); RecordingCollectorEnv env = DrawMetathesisRule(rule); + Assert.That(env.Text, Does.Contain("[Vd]"), "the left switch is drawn"); + Assert.That(env.Text, Does.Contain("[N]"), "the right switch is drawn"); Assert.That(NamesRegistered(env), Is.EquivalentTo(new[] { first.Hvo, second.Hvo })); } diff --git a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs index d09c7dc4a9..7ba2075b2c 100644 --- a/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs +++ b/Src/LexText/Morphology/MorphologyEditorDllTests/RuleFormulaVcTestBase.cs @@ -127,8 +127,7 @@ protected IPhSegRuleRHS AddRegularRule(string name) } /// - /// Adds a metathesis rule. Its structural change indices are left unset, so it holds - /// contexts without assigning any of them to a switch or environment cell. + /// Adds a metathesis rule with no contexts. /// protected IPhMetathesisRule AddMetathesisRule(string name) { @@ -139,14 +138,21 @@ protected IPhMetathesisRule AddMetathesisRule(string name) } /// - /// Adds a natural class simple context to a metathesis rule's structural description, - /// assigning the class once the context is owned. + /// Adds a natural class simple context to one cell of a metathesis rule the way the + /// formula editor does: into the structural description, then into the structural change + /// for that cell. The context is appended, so cells must be filled from left to right. /// - protected IPhSimpleContextNC AddStrucDescNCContext(IPhMetathesisRule rule, IPhNaturalClass natClass) + /// The rule to add the context to. + /// The cell, as a PhMetathesisRuleTags.kidx constant. + /// The class the context refers to. + protected IPhSimpleContextNC AddMetathesisNCContext(IPhMetathesisRule rule, int cellId, + IPhNaturalClass natClass) { IPhSimpleContextNC ctxt = Cache.ServiceLocator.GetInstance().Create(); - rule.StrucDescOS.Add(ctxt); + int index = rule.StrucDescOS.Count; + rule.StrucDescOS.Insert(index, ctxt); ctxt.FeatureStructureRA = natClass; + rule.UpdateStrucChange(cellId, index, true); return ctxt; }