Skip to content

Commit 8ea4500

Browse files
committed
feat: drop test-time dependency on registry-backed SilEncConverters40 EncConverters
- Replace direct static look‑ups of SilEncConverters40.EncConverters in unit tests with Moq fakes (IEncConverter/IEncConverters) and lazy‑init seams. - Guard EncConverters creation in SCTextEnum so it only occurs when a LegacyMapping is actually required, eliminating eager registry access. - Update affected Paratext import and Interlinear SFM import tests to use the new fakes and verify behavior without relying on global process/registry state. This removes non‑deterministic HKLM\Software\... registry dependencies from the test suite, making CI runs repeatable and allowing the code to be built/run in environments where those keys are absent.
1 parent e51824e commit 8ea4500

File tree

9 files changed

+147
-256
lines changed

9 files changed

+147
-256
lines changed

Src/FwCoreDlgs/CnvtrPropertiesCtrl.cs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -586,15 +586,10 @@ public void CnvtrPropertiesCtrl_Load(object sender, System.EventArgs e)
586586
{
587587
CheckDisposed();
588588

589-
// This is a fall-back if the creator does not have a converters object.
590-
// It is generally preferable for the creator to make one and pass it in.
591-
// Multiple EncConverters objects are problematical because they don't all get
592-
// updated when something changes.
593-
// JohnT: note that this ALWAYS happens at least once, because the Load event happens during
594-
// the main dialog's InitializeComponent method, before it sets the encConverters
595-
// of the control.
596-
if (m_encConverters == null)
597-
m_encConverters = new EncConverters();
589+
// Do not instantiate EncConverters here.
590+
// The creator normally supplies the instance after InitializeComponent; creating one
591+
// during Load can have undesirable side-effects (registry and repository IO) and can
592+
// break unit tests. Methods that truly require EncConverters should create it lazily.
598593
cboConverter.Items.Clear();
599594
cboConverter.Items.Add(new CnvtrTypeComboItem(AddConverterResources.kstrCc, ConverterType.ktypeCC, EncConverters.strTypeSILcc));
600595
if (!m_fOnlyUnicode)
@@ -636,6 +631,12 @@ public void CnvtrPropertiesCtrl_Load(object sender, System.EventArgs e)
636631
}
637632
}
638633

634+
private void EnsureEncConverters()
635+
{
636+
if (m_encConverters == null)
637+
m_encConverters = new EncConverters();
638+
}
639+
639640
/// ------------------------------------------------------------------------------------
640641
/// <summary>
641642
/// Handles the Click event of the btnMapFile control.
@@ -731,7 +732,9 @@ public void SelectMapping(string mapname)
731732
m_selectingMapping = true;
732733

733734
txtName.Text = mapname;
734-
IEncConverter conv = m_encConverters[mapname];
735+
IEncConverter conv = null;
736+
if (m_encConverters != null)
737+
conv = m_encConverters[mapname];
735738
ConvType convType;
736739
string implType;
737740
EncoderInfo undefinedEncoder = null; // in case the current selection is not fully defined
@@ -1018,6 +1021,8 @@ internal void SetStates(bool existingConvs, bool installedConverter)
10181021
/// ------------------------------------------------------------------------------------
10191022
private void btnModify_Click(object sender, EventArgs e)
10201023
{
1024+
EnsureEncConverters();
1025+
10211026
// call the v2.2 interface to "AutoConfigure" a converter
10221027
string strFriendlyName = ConverterName;
10231028
IEncConverter aEC = m_encConverters[strFriendlyName];
@@ -1125,6 +1130,8 @@ string strRhsEncodingID
11251130
/// ------------------------------------------------------------------------------------
11261131
private void AddToCollection(IEncConverter rConverter, string converterName)
11271132
{
1133+
EnsureEncConverters();
1134+
11281135
// now add it to the 'this' collection
11291136
// converterName.ToLower(); // this does nothing anyway, so get rid of it
11301137

Src/FwCoreDlgs/FwCoreDlgsTests/AdvancedScriptRegionVariantModelTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ public void GetStandardVariants_ListsLanguageSpecific()
293293
{
294294
var fwWsModel = new FwWritingSystemSetupModel(new TestWSContainer(new[] { "fr" }, new[] { "en" }), FwWritingSystemSetupModel.ListType.Vernacular);
295295
var model = new AdvancedScriptRegionVariantModel(fwWsModel);
296-
Assert.That("Early Modern French", Does.Contain(model.GetStandardVariants().Select(v => v.Name)));
296+
Assert.That(model.GetStandardVariants().Select(v => v.Name), Does.Contain("Early Modern French"));
297297
}
298298

299299
/// <summary/>
@@ -302,7 +302,7 @@ public void GetStandardVariants_ListsLanguageSpecificWithVariants()
302302
{
303303
var fwWsModel = new FwWritingSystemSetupModel(new TestWSContainer(new[] { "fr-x-extra" }, new[] { "en" }), FwWritingSystemSetupModel.ListType.Vernacular);
304304
var model = new AdvancedScriptRegionVariantModel(fwWsModel);
305-
Assert.That("Early Modern French", Does.Contain(model.GetStandardVariants().Select(v => v.Name)));
305+
Assert.That(model.GetStandardVariants().Select(v => v.Name), Does.Contain("Early Modern French"));
306306
}
307307

308308
/// <summary/>

Src/FwCoreDlgs/FwCoreDlgsTests/CnvtrPropertiesCtrlTests.cs

Lines changed: 11 additions & 160 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,6 @@ public void setCboConverter(ConverterType setTo)
169169
[TestFixture]
170170
public class CnvtrPropertiesControlTests
171171
{
172-
private DummyAddCnvtrDlg m_myDlg;
173172
private DummyCnvtrPropertiesCtrl m_myCtrl;
174173
private string m_ccFileName;
175174
private string m_mapFileName;
@@ -185,17 +184,8 @@ public class CnvtrPropertiesControlTests
185184
[OneTimeSetUp]
186185
public void FixtureSetup()
187186
{
188-
var encConverters = new EncConverters();
189-
// Remove any encoding converters we created that have been left over due to a crash
190-
// or other mishap. (That's why we use wierd names starting with ZZZUnitTest, so
191-
// there won't be any conceivable conflict with user chosen names. Inconceivable
192-
// conflicts might still happen, but...)
193-
RemoveTestConverters(encConverters, "Installed mappings before test setup:");
194187
string[] ccFileContents = {"'c' > 'C'"};
195188
m_ccFileName = CreateTempFile(ccFileContents, "cct");
196-
encConverters.AddConversionMap("ZZZUnitTestCC", m_ccFileName,
197-
ConvType.Legacy_to_Unicode, "SIL.cc", "", "",
198-
ProcessTypeFlags.UnicodeEncodingConversion);
199189

200190
string[] mapFileContents = {
201191
"EncodingName 'ZZZUnitTestText'",
@@ -205,41 +195,17 @@ public void FixtureSetup()
205195
"0x80 <> euro_sign"
206196
};
207197
m_mapFileName = CreateTempFile(mapFileContents, "map");
208-
encConverters.AddConversionMap("ZZZUnitTestMap", m_mapFileName,
209-
ConvType.Legacy_to_from_Unicode, "SIL.map", "", "",
210-
ProcessTypeFlags.UnicodeEncodingConversion);
211-
212-
// TODO: Should test a legitimate compiled TecKit file by embedding a zipped
213-
// up one in the resources for testing purposes.
214-
215-
// This is a randomly chosen ICU converter. The test may break when we reduce the set of
216-
// ICU converters we ship.
217-
encConverters.AddConversionMap("ZZZUnitTestICU", "ISO-8859-1",
218-
ConvType.Legacy_to_from_Unicode, "ICU.conv", "", "",
219-
ProcessTypeFlags.ICUConverter);
220-
221-
// Add a 1-step compound converter, which won't be any of the types our dialog
222-
// recognizes for now.
223-
encConverters.AddCompoundConverterStep("ZZZUnitTestCompound", "ZZZUnitTestCC", true,
224-
NormalizeFlags.None);
225-
226-
encConverters.Remove("BogusTecKitFile"); // shouldn't exist, but...
227-
228-
m_myDlg = new DummyAddCnvtrDlg();
229198
m_myCtrl = new DummyCnvtrPropertiesCtrl();
230-
m_myCtrl.Converters = encConverters;
231-
// Load all the mappings after the dummy mappings are added, so the Converter
232-
// Mapping File combo box won't contain obsolete versions of the mappings referring
233-
// to old temp files from a previous run of the tests.q
234-
m_myCtrl.CnvtrPropertiesCtrl_Load(null, null);
235-
#if !QUIET
236-
Console.WriteLine("Installed mappings after test setup:");
237-
foreach (var name in encConverters.Mappings)
199+
m_myCtrl.UndefinedConverters = new System.Collections.Generic.Dictionary<string, EncoderInfo>
238200
{
239-
var conv = encConverters[name];
240-
Console.WriteLine(" {0} ({1})", name, conv == null ? "null" : conv.GetType().ToString());
241-
}
242-
#endif
201+
{ "ZZZUnitTestCC", new EncoderInfo("ZZZUnitTestCC", ConverterType.ktypeCC, m_ccFileName, ConvType.Legacy_to_Unicode) },
202+
{ "ZZZUnitTestMap", new EncoderInfo("ZZZUnitTestMap", ConverterType.ktypeTecKitMap, m_mapFileName, ConvType.Legacy_to_from_Unicode) },
203+
{ "ZZZUnitTestICU", new EncoderInfo("ZZZUnitTestICU", ConverterType.ktypeIcuConvert, "ISO-8859-1", ConvType.Legacy_to_from_Unicode) },
204+
// Use an unknown ConverterType value so it won't match any known ImplementType.
205+
{ "ZZZUnitTestCompound", new EncoderInfo("ZZZUnitTestCompound", (ConverterType)9999, "", ConvType.Legacy_to_from_Unicode) },
206+
};
207+
// Populate UI comboboxes without touching EncConverters.
208+
m_myCtrl.CnvtrPropertiesCtrl_Load(null, null);
243209
}
244210

245211
/// <summary>
@@ -248,24 +214,11 @@ public void FixtureSetup()
248214
[OneTimeTearDown]
249215
public void FixtureTeardown()
250216
{
251-
EncConverters encConverters;
252-
// Dispose managed resources here.
253217
if (m_myCtrl != null)
254218
{
255-
encConverters = m_myCtrl.Converters;
256219
m_myCtrl.Dispose();
257220
m_myCtrl = null;
258221
}
259-
else
260-
{
261-
encConverters = new EncConverters();
262-
}
263-
264-
if (m_myDlg != null)
265-
{
266-
m_myDlg.Dispose();
267-
m_myDlg = null;
268-
}
269222

270223
try
271224
{
@@ -291,25 +244,6 @@ public void FixtureTeardown()
291244
// for some reason deleting the temporary files occasionally fails - not sure
292245
// why. If this happens we just ignore it and continue.
293246
}
294-
295-
// Remove any encoding converters that we may have created during this test run.
296-
RemoveTestConverters(encConverters, "Installed mappings after test teardown:");
297-
}
298-
299-
void RemoveTestConverters(EncConverters encConverters, string testMessage)
300-
{
301-
// Remove any encoding converters that were added for these tests.
302-
encConverters.Remove("ZZZUnitTestCC");
303-
encConverters.Remove("ZZZUnitTestText");
304-
encConverters.Remove("ZZZUnitTestMap");
305-
encConverters.Remove("ZZZUnitTestICU");
306-
encConverters.Remove("ZZZUnitTestCompound");
307-
encConverters.Remove("ZZZUnitTestBogusTecKitFile"); // shouldn't exist, but...
308-
#if !QUIET
309-
Console.WriteLine("{0}", testMessage);
310-
foreach (var name in encConverters.Mappings)
311-
Console.WriteLine(" {0}", name);
312-
#endif
313247
}
314248
#endregion
315249

@@ -378,7 +312,6 @@ public void SelectMapping_TecKitMapTable()
378312
[Test]
379313
public void SelectMapping_IcuConversion()
380314
{
381-
var encConverterStoredType = m_myCtrl.Converters.GetMapByName("ZZZUnitTestICU").ConversionType;
382315
m_myCtrl.SelectMapping("ZZZUnitTestICU");
383316
Assert.That(m_myCtrl.cboConverter.SelectedItem is CnvtrTypeComboItem, Is.True, "Should be able to select ZZZUnitTestICU");
384317
Assert.That(((CnvtrTypeComboItem)m_myCtrl.cboConverter.SelectedItem).Type, Is.EqualTo(ConverterType.ktypeIcuConvert), "Selected item should be ICU converter for ZZZUnitTestICU");
@@ -390,7 +323,7 @@ public void SelectMapping_IcuConversion()
390323
// ICU converters we ship.
391324
Assert.That(((CnvtrSpecComboItem)m_myCtrl.cboSpec.SelectedItem).Specs, Is.EqualTo("ISO-8859-1"), "Selected spec should be ISO-8859-1 for ZZZUnitTestICU");
392325
Assert.That(m_myCtrl.cboConversion.SelectedItem is CnvtrDataComboItem, Is.True, "Conversion type should be selected for ZZZUnitTestICU");
393-
Assert.That(((CnvtrDataComboItem)m_myCtrl.cboConversion.SelectedItem).Type, Is.EqualTo(encConverterStoredType), "Selected Conversion type should match the value stored in EncConverters for ZZZUnitTestICU");
326+
Assert.That(((CnvtrDataComboItem)m_myCtrl.cboConversion.SelectedItem).Type, Is.EqualTo(ConvType.Legacy_to_from_Unicode), "Selected Conversion type should match the value stored for ZZZUnitTestICU");
394327
Assert.That(m_myCtrl.txtName.Text, Is.EqualTo("ZZZUnitTestICU"), "Displayed converter should be ZZZUnitTestICU");
395328
}
396329

@@ -498,88 +431,6 @@ public void SelectMapping_PrepopulateCboConversion()
498431
Assert.That(((CnvtrDataComboItem)m_myCtrl.cboConversion.SelectedItem).Type, Is.EqualTo(ConvType.Legacy_to_from_Unicode), "CodePage type defaults to Legacy_to_from_Unicode");
499432
}
500433

501-
/// ------------------------------------------------------------------------------------
502-
/// <summary>
503-
/// Test a bogus compiled TecKit file. Should fail with nice error message, not crash.
504-
/// </summary>
505-
/// ------------------------------------------------------------------------------------
506-
[Test]
507-
public void SelectMapping_BogusCompiledTecKitFile()
508-
{
509-
m_bogusFileName = CreateTempFile(new string[] { "bogus contents" }, "tec");
510-
511-
// This is a type we don't recognize.
512-
m_myDlg.m_cnvtrPropertiesCtrl.txtName.Text = "ZZZUnitTestBogusTecKitFile";
513-
514-
int i;
515-
for (i = 0; i < m_myDlg.m_cnvtrPropertiesCtrl.cboConverter.Items.Count; ++i)
516-
{
517-
if (((CnvtrTypeComboItem)m_myDlg.m_cnvtrPropertiesCtrl.cboConverter.Items[i]).Type == ConverterType.ktypeTecKitTec)
518-
{
519-
m_myDlg.m_cnvtrPropertiesCtrl.cboConverter.SelectedIndex = i;
520-
break;
521-
}
522-
}
523-
Assert.That(i < m_myDlg.m_cnvtrPropertiesCtrl.cboConverter.Items.Count, Is.True, "Should find a TecKitTec type converter listed.");
524-
for (i = 0; i < m_myDlg.m_cnvtrPropertiesCtrl.cboConversion.Items.Count; ++i)
525-
{
526-
if (((CnvtrDataComboItem)m_myDlg.m_cnvtrPropertiesCtrl.cboConversion.Items[i]).Type == ConvType.Legacy_to_Unicode)
527-
{
528-
m_myDlg.m_cnvtrPropertiesCtrl.cboConversion.SelectedIndex = i;
529-
break;
530-
}
531-
}
532-
Assert.That(i < m_myDlg.m_cnvtrPropertiesCtrl.cboConversion.Items.Count, Is.True, "Should find a Legacy_to_Unicode conversion listed.");
533-
534-
m_myDlg.SetMappingFile(m_bogusFileName);
535-
536-
Assert.That(m_myDlg.InstallConverter(), Is.False, "Should not be able to install bogus compiled TecKit file.");
537-
// This may not be testing what we want it to test...
538-
// Might want make an assert on the error message that is produced!
539-
}
540-
541-
/// ------------------------------------------------------------------------------------
542-
/// <summary>
543-
/// Tests for a "successful" save when nothing is changed
544-
/// </summary>
545-
/// ------------------------------------------------------------------------------------
546-
[Test]
547-
public void AutoSave_ValidButUnchanged()
548-
{
549-
m_myDlg.m_cnvtrPropertiesCtrl.SelectMapping("ZZZUnitTestCC");
550-
m_myDlg.SetUnchanged();
551-
Assert.That(m_myDlg.AutoSave(), Is.True);
552-
}
553-
554-
/// ------------------------------------------------------------------------------------
555-
/// <summary>
556-
/// Tests for a successful save when converter is valid
557-
/// </summary>
558-
/// ------------------------------------------------------------------------------------
559-
[Test]
560-
[Ignore("Fails about half of the time -- CameronB")]
561-
public void AutoSave_ValidContents()
562-
{
563-
m_myDlg.m_cnvtrPropertiesCtrl.SelectMapping("ZZZUnitTestICU");
564-
m_myDlg.SetUnchanged();
565-
m_myDlg.m_cnvtrPropertiesCtrl.txtName.Text = "ZZZUnitTestRenamedICU";
566-
Assert.That(m_myDlg.AutoSave(), Is.True);
567-
}
568-
569-
/// ------------------------------------------------------------------------------------
570-
/// <summary>
571-
/// Tests for failure when converter cannot save successfully
572-
/// </summary>
573-
/// ------------------------------------------------------------------------------------
574-
[Test]
575-
[Ignore("Fails: producing object null message")]
576-
public void AutoSave_InvalidContents()
577-
{
578-
m_myDlg.m_cnvtrPropertiesCtrl.SelectMapping("ZZZUnitTestMap");
579-
m_myDlg.SetUnchanged();
580-
m_myDlg.m_cnvtrPropertiesCtrl.cboSpec.Text = "NotValid";
581-
Assert.That(m_myDlg.AutoSave(), Is.False);
582-
}
583434
#endregion
584435

585436
/// <summary>
@@ -593,7 +444,7 @@ string CreateTempFile(string[] data, string filetype)
593444
{
594445
filename = Path.ChangeExtension(fileTmp, filetype);
595446
File.Move(fileTmp, filename);
596-
}
447+
}
597448
using (var file = new StreamWriter(filename, false, System.Text.Encoding.ASCII))
598449
{
599450
foreach (var line in data)

Src/LexText/Interlinear/ITextDllTests/InterlinSfmImportTests.cs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
using System.Xml.Linq;
1010
using System.Xml.XPath;
1111
using NUnit.Framework;
12+
using Moq;
1213
using Sfm2Xml;
1314
using SIL.FieldWorks.LexText.Controls;
1415
using SilEncConverters40;
16+
using ECInterfaces;
1517
using SIL.LCModel.Core.WritingSystems;
1618

1719
namespace SIL.FieldWorks.IText
@@ -165,10 +167,15 @@ public void BasicConversion()
165167
[Test]
166168
public void EncodingConverters()
167169
{
168-
var encConv = new EncConverters();
169-
encConv.AddConversionMap("XXYTestConverter", "1252",
170-
ECInterfaces.ConvType.Legacy_to_from_Unicode, "cp", "", "",
171-
ECInterfaces.ProcessTypeFlags.CodePageConversion);
170+
var mockConverter = new Mock<IEncConverter>();
171+
mockConverter
172+
.Setup(c => c.ConvertToUnicode(It.IsAny<byte[]>()))
173+
.Returns((byte[] bytes) => Encoding.GetEncoding(1252).GetString(bytes));
174+
175+
var mockEncConverters = new Mock<IEncConverters>();
176+
mockEncConverters.Setup(c => c[It.IsAny<string>()]).Returns((IEncConverter)null);
177+
mockEncConverters.Setup(c => c["XXYTestConverter"]).Returns(mockConverter.Object);
178+
172179
var mappings = new List<InterlinearMapping>();
173180
mappings.Add(new InterlinearMapping()
174181
{
@@ -186,7 +193,7 @@ public void EncodingConverters()
186193
});
187194
var wsf = GetWsf();
188195
var input = new ByteReader("input2", Encoding.GetEncoding(1252).GetBytes(input2));
189-
var converter = new Sfm2FlexText();
196+
var converter = new Sfm2FlexText(mockEncConverters.Object);
190197
var output = converter.Convert(input, mappings, wsf);
191198
using (var outputStream = new MemoryStream(output))
192199
{
@@ -198,7 +205,6 @@ public void EncodingConverters()
198205
var phrase1 = textElt.XPathSelectElement("./paragraphs/paragraph/phrases/phrase");
199206
VerifyText(phrase1, new[] { "John", "added", "this\x017D" },
200207
new HashSet<string>(), "qaa-x-kal");
201-
encConv.Remove("XXYTestConverter");
202208
}
203209
}
204210
}

Src/LexText/Interlinear/Sfm2FlexText.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System;
66
using System.Collections.Generic;
77
using System.Linq;
8+
using ECInterfaces;
89
using SIL.LCModel.Core.Text;
910
using SIL.FieldWorks.LexText.Controls;
1011
using SIL.LCModel.DomainServices;
@@ -33,6 +34,11 @@ internal Sfm2FlexText() : base (new List<string>(new [] { "document", "interline
3334
{
3435
}
3536

37+
internal Sfm2FlexText(IEncConverters encConverters) : base(new List<string>(new[] { "document", "interlinear-text",
38+
"paragraphs", "paragraph", "phrases", "phrase", "words", "word" }), encConverters)
39+
{
40+
}
41+
3642
protected override void WriteToDocElement(byte[] data, InterlinearMapping mapping)
3743
{
3844
switch (mapping.Destination)

0 commit comments

Comments
 (0)