Skip to content

Commit d194d06

Browse files
committed
tighten RealFLASHDeconvolution test assertions and fill parser gaps
Group A of the PR #1045 review pass. - TestRealFLASHDeconvolutionInfrastructure: tighten the EnvelopeCountMatchesReference assertion from "> 1000" to "Is.EqualTo(3149).Within(150)" so a regression cutting the output by half no longer silently passes -- matching the test name's promise. Comment documents the calibration to OpenMS-3.0.0-pre-HEAD-2023-06-17 so contributors on a different version know to either update the constant or run with FLASHDECONV_EXE pointed at the reference build. - Replace Assume.That(allEnvs.Count, Is.GreaterThan(0)) with Assert.That in EnvelopeFieldsValid and MassesInRange so a zero-envelope regression is a hard failure, not a silent Inconclusive that gets waved off in CI noise. - Add ParseSpecTsvByScan_HeaderOnly_ReturnsEmptyDictionary covering the lines.Length < 2 early-return. - Add ParseSpecTsvByScan_RowsWithUnparseableRequiredCells_AreSilentlySkipped pinning the per-row "skip cell-parse failures, return what's valid" contract for real FLASHDeconv output that contains "NA" cells. - Add Deconvolute_StubRunner_RangeFilterIsInclusiveOnUpperBound pinning the boundary semantic of MzRange.Contains (which DoubleRange documents as inclusive) at the upper edge. - Add Deconvolute_EmptySpectrum_RunnerIsNotInvoked using the new FLASHDeconvRunner injection to assert directly that the runner is never invoked on an empty spectrum, instead of relying on the indirect side effect of an unresolvable exe path (the existing companion test).
1 parent 47cbfc0 commit d194d06

1 file changed

Lines changed: 155 additions & 6 deletions

File tree

mzLib/Test/Deconvolution/TestRealFLASHDeconvolutionInfrastructure.cs

Lines changed: 155 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,31 @@ public void RealFLASH_Factory_EmptySpectrum_ReturnsEmptyWithoutCallingExe()
113113
}, Throws.Nothing);
114114
}
115115

116+
[Test]
117+
public void Deconvolute_EmptySpectrum_RunnerIsNotInvoked()
118+
{
119+
// Pins the empty-spectrum short-circuit DIRECTLY via a stub runner that
120+
// fails the test if invoked, instead of relying on the indirect side
121+
// effect of an unresolvable exe path. A future refactor that moves or
122+
// restructures ResolveExePath could quietly cause _runner(...) to be
123+
// called on empty spectra without breaking the indirect test above.
124+
string fakeExe = CreateFakeExeFile();
125+
try
126+
{
127+
var p = new RealFLASHDeconvolutionParameters(flashDeconvExePath: fakeExe);
128+
RealFLASHDeconvolutionAlgorithm.FLASHDeconvRunner failIfCalled =
129+
(_, _, _, _, _, _) => Assert.Fail("Runner must not be invoked for an empty spectrum.");
130+
131+
var algorithm = new RealFLASHDeconvolutionAlgorithm(p, failIfCalled);
132+
var empty = new MzSpectrum(Array.Empty<double>(), Array.Empty<double>(), shouldCopy: false);
133+
134+
var result = algorithm.Deconvolute(empty, new MzRange(0, 5000)).ToList();
135+
136+
Assert.That(result, Is.Empty);
137+
}
138+
finally { File.Delete(fakeExe); }
139+
}
140+
116141
[Test]
117142
public void RealFLASH_Factory_BadExePath_ThrowsFileNotFound()
118143
{
@@ -213,10 +238,20 @@ public void RealFLASH_RealExe_WholeFile_SmallYeast_EnvelopeCountMatchesReference
213238
Console.WriteLine($"Total envelopes: {total}");
214239

215240
// Reference run on OpenMS-3.0.0-pre-HEAD-2023-06-17 produced 3149 envelopes.
216-
// We assert a sanity floor rather than the exact count so the test stays
217-
// green across OpenMS versions.
218-
Assert.That(total, Is.GreaterThan(1000),
219-
"Envelope count should be substantial (reference run produced 3149).");
241+
// ~5% tolerance is generous enough to absorb legitimate cross-version drift
242+
// (minor OpenMS bumps, scoring-threshold tweaks) but tight enough that a
243+
// regression cutting the output by half would no longer silently pass --
244+
// matching what the test name "MatchesReference" promises.
245+
//
246+
// The 3149 constant is calibrated specifically to OpenMS-3.0.0-pre-HEAD-2023-06-17.
247+
// Contributors who run this test against a newer OpenMS (3.3.0 / 3.4.0 / 3.5.0,
248+
// all of which appear in the algorithm's well-known-paths search) may see the
249+
// count drift outside ±150 due to upstream filter or scoring-threshold tweaks
250+
// -- this is an environment difference, not an mzLib regression. Either update
251+
// the constant after verifying the new count is plausible, or run with
252+
// FLASHDECONV_EXE pointed at the reference version.
253+
Assert.That(total, Is.EqualTo(3149).Within(150),
254+
"Envelope count must be within 5% of the reference run (3149) to catch real regressions.");
220255
}
221256

222257
[Test]
@@ -234,7 +269,8 @@ public void RealFLASH_RealExe_WholeFile_SmallYeast_EnvelopeFieldsValid()
234269
var allEnvs = RealFLASHDeconvolutionAlgorithm.DeconvoluteFile(mzml, p)
235270
.Values.SelectMany(x => x).ToList();
236271

237-
Assume.That(allEnvs.Count, Is.GreaterThan(0));
272+
Assert.That(allEnvs.Count, Is.GreaterThan(0),
273+
"Regression that empties FLASHDeconv output should be a hard failure, not Inconclusive");
238274

239275
foreach (var env in allEnvs)
240276
{
@@ -261,7 +297,8 @@ public void RealFLASH_RealExe_WholeFile_SmallYeast_MassesInRange()
261297
var allEnvs = RealFLASHDeconvolutionAlgorithm.DeconvoluteFile(mzml, p)
262298
.Values.SelectMany(x => x).ToList();
263299

264-
Assume.That(allEnvs.Count, Is.GreaterThan(0));
300+
Assert.That(allEnvs.Count, Is.GreaterThan(0),
301+
"Regression that empties FLASHDeconv output should be a hard failure, not Inconclusive");
265302

266303
// All masses must be within the requested bounds
267304
Assert.That(allEnvs.All(e => e.MonoisotopicMass >= 50 && e.MonoisotopicMass <= 5000),
@@ -489,6 +526,78 @@ public void ParseSpecTsvByScan_OptionalColumnsAbsent_ParsesAndUsesFallbacks()
489526
}
490527
}
491528

529+
[Test]
530+
public void ParseSpecTsvByScan_HeaderOnly_ReturnsEmptyDictionary()
531+
{
532+
// Pins the lines.Length < 2 early-return: FLASHDeconv occasionally
533+
// produces an MS1 TSV with only the header row when filters reject
534+
// every candidate envelope. The parser must return an empty dict
535+
// cleanly rather than throw or proceed into the row loop.
536+
string tsv = Path.Combine(Path.GetTempPath(),
537+
$"realflash_headeronly_{Guid.NewGuid():N}.tsv");
538+
try
539+
{
540+
File.WriteAllLines(tsv, new[]
541+
{
542+
"ScanNum\tMonoisotopicMass\tSumIntensity\tRepresentativeCharge\t" +
543+
"RepresentativeMzStart\tRepresentativeMzEnd\tIsotopeCosine\tMassSNR\tQscore",
544+
});
545+
546+
var p = new RealFLASHDeconvolutionParameters(
547+
flashDeconvExePath: @"C:\unused\FLASHDeconv.exe",
548+
minMass: 50, maxMass: 5000);
549+
550+
var result = RealFLASHDeconvolutionAlgorithm.ParseSpecTsvByScan(tsv, p);
551+
552+
Assert.That(result, Is.Empty);
553+
}
554+
finally
555+
{
556+
if (File.Exists(tsv)) File.Delete(tsv);
557+
}
558+
}
559+
560+
[Test]
561+
public void ParseSpecTsvByScan_RowsWithUnparseableRequiredCells_AreSilentlySkipped()
562+
{
563+
// Pins the per-row silent-skip cascade: real FLASHDeconv output occasionally
564+
// contains "NA" or other non-numeric cells in required columns (ScanNum,
565+
// MonoisotopicMass, RepresentativeCharge, ...). The parser's documented contract
566+
// is "skip the row, return what you can" -- not "throw" and not "include with
567+
// garbage values." Mix valid + unparseable rows in one file and verify only
568+
// the valid ones come back.
569+
string tsv = Path.Combine(Path.GetTempPath(),
570+
$"realflash_unparseable_{Guid.NewGuid():N}.tsv");
571+
try
572+
{
573+
File.WriteAllLines(tsv, new[]
574+
{
575+
"ScanNum\tMonoisotopicMass\tSumIntensity\tRepresentativeCharge\t" +
576+
"RepresentativeMzStart\tRepresentativeMzEnd\tIsotopeCosine\tMassSNR\tQscore",
577+
"1\t1000.0\t5.0e5\t5\t201.0\t201.4\t0.95\t12.0\t0.8", // valid
578+
"NA\t2000.0\t6.0e5\t8\t251.5\t251.9\t0.88\t14.0\t0.7", // bad ScanNum
579+
"2\tNA\t6.0e5\t8\t251.5\t251.9\t0.88\t14.0\t0.7", // bad mono
580+
"3\t1500.0\t4.5e5\tNA\t251.0\t251.3\t0.90\t11.0\t0.75", // bad charge
581+
"4\t1800.0\t7.0e5\t6\t301.0\t301.4\t0.92\t13.0\t0.85", // valid
582+
});
583+
584+
var p = new RealFLASHDeconvolutionParameters(
585+
flashDeconvExePath: @"C:\unused\FLASHDeconv.exe",
586+
minMass: 50, maxMass: 5000);
587+
588+
var byScan = RealFLASHDeconvolutionAlgorithm.ParseSpecTsvByScan(tsv, p);
589+
590+
int total = byScan.Values.Sum(l => l.Count);
591+
Assert.That(total, Is.EqualTo(2),
592+
"Only the two well-formed rows should have parsed; the three with unparseable required cells must be silently skipped.");
593+
Assert.That(byScan.Keys, Is.EquivalentTo(new[] { 1, 4 }));
594+
}
595+
finally
596+
{
597+
if (File.Exists(tsv)) File.Delete(tsv);
598+
}
599+
}
600+
492601
// ── E: ResolveExePath unit tests ──────────────────────────────────────
493602

494603
[Test]
@@ -631,6 +740,46 @@ public void Deconvolute_StubRunner_FiltersEnvelopesByRange()
631740
finally { File.Delete(fakeExe); }
632741
}
633742

743+
[Test]
744+
public void Deconvolute_StubRunner_RangeFilterIsInclusiveOnUpperBound()
745+
{
746+
// Pins the boundary semantic: MzRange.Contains is inclusive on both ends
747+
// (DoubleRange.Contains, "True if the item is within the range (inclusive)"),
748+
// so an envelope whose representative-mz midpoint sits EXACTLY at the upper
749+
// bound must be kept, and one that's epsilon past it must be dropped.
750+
// Without this, a future change to MzRange's endpoint behaviour or to the
751+
// Where-clause expression could silently shift edge results.
752+
string fakeExe = CreateFakeExeFile();
753+
try
754+
{
755+
var p = new RealFLASHDeconvolutionParameters(
756+
flashDeconvExePath: fakeExe,
757+
minMass: 50, maxMass: 100_000);
758+
759+
// Three rows: midpoint exactly at 1000.0 (must be IN), midpoint just past
760+
// (1000.05, must be OUT), and clearly in (200.0) as a sanity baseline.
761+
// Midpoint = (mzStart + mzEnd) / 2.
762+
string tsv = string.Join("\n",
763+
Ms1TsvHeader,
764+
"1\t1000.0\t5.0e5\t5\t999.9\t1000.1\t0.95\t12.0\t0.8", // mid=1000.0, IN (inclusive)
765+
"1\t2000.0\t6.0e5\t8\t999.9\t1000.2\t0.88\t14.0\t0.7", // mid=1000.05, OUT
766+
"1\t3000.0\t7.0e5\t6\t199.5\t200.5\t0.92\t13.0\t0.85"); // mid=200.0, IN baseline
767+
768+
var algorithm = new RealFLASHDeconvolutionAlgorithm(p, BuildStubRunner(tsv));
769+
770+
var result = algorithm
771+
.Deconvolute(BuildSyntheticSpectrum(), new MzRange(0, 1000))
772+
.ToList();
773+
774+
Assert.That(result, Has.Count.EqualTo(2),
775+
"Range filter is inclusive on the upper bound: 1000.0 is IN, 1000.05 is OUT, baseline is IN.");
776+
Assert.That(result.Select(e => e.MonoisotopicMass).OrderBy(m => m),
777+
Is.EqualTo(new[] { 1000.0, 3000.0 }).AsCollection,
778+
"Expected the at-bound and baseline envelopes; the just-past envelope must be dropped.");
779+
}
780+
finally { File.Delete(fakeExe); }
781+
}
782+
634783
[Test]
635784
public void DeconvoluteFile_StubRunnerWritesValidTsv_ReturnsScansKeyedByScanNum()
636785
{

0 commit comments

Comments
 (0)