Skyline: Improve failure reporting in AssertEx.FileEquals

[brendanx67]

This greatly improves failure reporting for AssertEx.FileEquals() where the old code used to just report the two lines where the comparison failed, the new code gives a lot more detail on the internal reasons when paths and column tolerances are involved. This code also adds a class ColumnTolerances to formalize the tolerance evaluation and introduces a tolerance that applies to the mantissa of the number in scientific notation to better capture limitations in precision and potential rounding error.

Before (mistakenly interpreted as a path difference):

Microsoft.VisualStudio.TestTools.UnitTesting.AssertFailedException: Assert.Fail failed. Diff found at line 225 position 0: expected
Orbi3_SA_IP_pHis3_01.mzML 17836 17876 8 3 3495.7076 1166.5775 1.050319E+08 2.961274E+08 110.6513 110.8777 110.7155 0.9989 _ 17848 H254C156N43O46S1[-2.150306] mass3495.7076_RT110.7155
actual
z:\download\Perftests\Label-free\converted\Orbi3_SA_IP_pHis3_01.mzML 17836 17876 8 3 3495.7076 1166.5775 1.050319E+08 2.961275E+08 110.6513 110.8777 110.7155 0.9989 _ 17848 H254C156N43O46S1[-2.150306] mass3495.7076_RT110.7155

After (skipping thousands of lines that match by the specified precision and more clearly showing the problem):

Microsoft.VisualStudio.TestTools.UnitTesting.AssertFailedException: Assert.Fail failed. Diff found at line 7723 position 5:
expected
Orbi3_SA_IP_pHis3_01.mzML 2350 2371 5 1 863.4852 864.4928 1732674 5650320 15.8053 15.9211 15.8631 0.9988 _ 2361 H86C38N10O11[+4.837413] mass863.4852_RT15.8631
actual
z:\download\Perftests\Label-free\converted\Orbi3_SA_IP_pHis3_01.mzML 8456 8477 6 2 1228.6413 615.3279 1732675 8681932 51.0093 51.1453 51.1453 0.9942 _ 8477 H102C55N15O16[-0.121619] mass1228.6413_RT51.1453
expected with paths removed
path 2350 2371 5 1 863.4852 864.4928 1732674 5650320 15.8053 15.9211 15.8631 0.9988 _ 2361 H86C38N10O11[+4.837413] mass863.4852_RT15.8631
actual with paths removed
path 8456 8477 6 2 1228.6413 615.3279 1732675 8681932 51.0093 51.1453 51.1453 0.9942 _ 8477 H102C55N15O16[-0.121619] mass1228.6413_RT51.1453
matching prefix: 'path '
matching suffix: ''
decimal matching: Expected decimal mantissa: 2.35 does not match actual 8.456 to within 0.00015015

[bspratt]

Looks good, other than I really do not think we should be using the scientific notation logic on integers. It took me a little while to understand that it was complaining about "2350" vs "8456". When I think about it, I'd argue for using the E logic only if one or the other text representations does in fact use scientific notation.

And it seems like we could be reporting the column index when comparing TSVs, but that's a nice-to-have.

[brendanx67]

I think the integer issue can still be a precision thing. I did realize that I could be reporting the original text and the delta multiplied by the exponent which would be clearer in the integer case. Maybe I will do that before merging. Just wanted to get this code in a PR, because I felt it helped immensely to understand what was going on, and in the future avoid thinking about paths when they were not the issue.

…

On Wed, Jun 19, 2024 at 2:13 PM Brian Pratt ***@***.***> wrote: Looks good, other than I really do not think we should be using the scientific notation logic on integers. It took me a little while to understand that it was complaining about "2350" vs "8456". When I think about it, I'd argue for using the E logic only if one or the other text representations does in fact use scientific notation. And it seems like we could be reporting the column index when comparing TSVs, but that's a nice-to-have. — Reply to this email directly, view it on GitHub <#3021 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACYBWUCHBOJEJKUMU6LRZEDZIHYARAVCNFSM6AAAAABJSVHKGCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZZGQ3DENZVG4> . You are receiving this because you authored the thread.Message ID: ***@***.***>