NH-4028 - Supports inconclusive tests in result comparison #641
Conversation
fredericDelaporte
force-pushed
the
NH-4028
branch
from
b350aae to
2c8c5c5
Compare
| { | ||
| get | ||
| { | ||
| return Executed && !Success && Status == ResultStatus.Success; |
There was a problem hiding this comment.
Issuing a NUnit warning causes the test to have Success as result attribute value with false as success attribute value. So I have added an Inconclusive property matching these on the Result class.
teamcity.build
Outdated
| report.AppendLine("=================="); | ||
|
|
||
| var before = Result.ParseFile(lastResult); | ||
| var beforeByName = before.GroupBy(b => b.Name).ToDictionary(g => g.Key, g => g.First()); |
There was a problem hiding this comment.
Some existing result file have duplicated tests. The previous code was matching new tests to each of them, with those new tests ending up potentially duplicated in the results lists below.
Here I simply take the first. The dup test is currently always NHibernate.Test.Linq.WhereTests.CanUseCompareInQuery(p => (p.ShippingWeight.CompareTo(4.98) <= 0),17,False). I am going to fix its test case, it does it twice. All outcome were same, but just in case, I will change the logic above for considering non-failing results first.
teamcity.build
Outdated
| var ignoredTests = new List<Result>(); | ||
| var brokenTests = new List<Result>(); | ||
| var inconclusiveTests = new List<Result>(); | ||
| foreach (var afterResult in after) |
There was a problem hiding this comment.
Now aggregating results in one pass. Previous was looping on after, then on before for matching, then adding in one result list, and start over for the next list... With the growing number of tests, a O(n²) logic may end up some day taking a significant duration.
| report.AppendLine("None"); | ||
|
|
||
| report.AppendLine(); | ||
| report.AppendLine("*** Tests new (failed) since last recorded results ***"); |
There was a problem hiding this comment.
I have moved the failed tests at start of file. Seeking them in the middle of other was not very practical.
fredericDelaporte
force-pushed
the
NH-4028
branch
from
142fe3a to
0cd40d2
Compare
…ups, slightly better comparison ordering.
fredericDelaporte
force-pushed
the
NH-4028
branch
from
0cd40d2 to
3ccf326
Compare
|
Any thoughts about this? That is probably the last piece missing for ending my work on #627. (Since I will not get Firebird into dtc tests unless at least one of its data provider issues (DNET-764, DNET-766) is fixed fast.) Of course I may instead give up that "inconclusive" status instead and ignore corresponding tests for Oracle, but that will cause Oracle to not be thoroughly tested with DTC. And if we do that, I think I will keep this PR, expurge from that inconclusive status, but keeping the other changes done for avoiding a O(n²) logic with |
NH-4028 - Supports inconclusive tests in result comparison
Within #627, I have started issuing NUnit warning about problematic database behaviors we cannot fix. This causes the test to be inconclusive: it executes without errors but exhibit abnormal behaviors.
Current comparison result logic classifies them as failing. Teamcity classifies them as succeeded. The build log contains their warning.
Adjust comparison result logic to classify them as inconclusive and report them accordingly in comparison result. Do not cause the build to fail on them.
An example of such behavior causing a test to be inconclusive is the delayed commit with Oracle when the transaction is distributed, occurring after transaction scope disposal, instead of being committed before the disposal is left. The commit succeeded, but a query done right after the scope may not be able to see it. The test in #627 sleep a bit to check if the data finally gets committed. If no, it fails, if yes, it warns about the delayed commit, causing the test to be inconclusive.