-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce a TestSummary #53
Conversation
…9/Tasty into topic/report-helpers-51
…hazard999/Tasty into topic/report-helpers-51" This reverts commit 9c60366, reversing changes made to 8629308.
result[outcome.Key] = new TestResult(outcome.Value.Count, outcome.Value.Sum(t => t.Duration)); | ||
} | ||
|
||
OutCome = tests.Where(t => t.TestOutcome > TestOutcome.Ignored).MinOrDefault(t => t.TestOutcome); ; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
; ;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Casing:
OutCome -> Outcome
testsPerOutCome -> testsPerOutcome
@@ -17,11 +16,25 @@ public static TestExecutor UseSummaryReporters(this TestExecutor executor) | |||
} | |||
finally | |||
{ | |||
var cases = context.Scope.Descendants().OfType<TestCase>().ToList(); | |||
async Task<IEnumerable<TestSummary>> Summaries() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use an IAsyncEnumerable
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible, but I couldn't make it work properly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public static class ReportSummaryMiddleware
{
public static TestExecutor UseSummaryReporters(this TestExecutor executor)
=> executor.UseRuntime(async (context, next) =>
{
try
{
await next();
}
finally
{
async IAsyncEnumerable<TestSummary> Summaries()
{
foreach (var provider in context.Scope.SummaryProviders)
{
yield return await provider.Invoke();
}
}
await Task.WhenAll(context.Scope.SummaryReporters
.Select(async r =>
{
await foreach (var summary in Summaries())
{
await r.Invoke(summary);
}
}).ToArray());
}
});
}
Task SummaryReporter(IEnumerable<Metadata.TestCase> @cases) | ||
=> remote.Report(@cases.Select(test => MapToSerializableTestCase(test))); | ||
Task SummaryReporter(TestSummary summary) | ||
=> remote.Report(summary.Select(test => MapToSerializableTestCase(test))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we change the signature of remote summary reporters in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?!?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this form remote reporters still do their stuff on their own and won't get any profit for now, cause it's IEnumerable<SerializableTestCase>
over the wire. So we can change that to an SerializableTestSummary
in the future.
|
||
namespace Xenial.Delicious.Reporters | ||
{ | ||
public static class SummaryReporter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about this type. The pipeline only pumps trough collected tests to the original summary reporters anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it would be something like -> AsyncTestReporters -> All Tests done in the pipeline -> AsyncSummaryCollectors? -> AsyncSummaryReporters? in the pipeline? Sounds like something we should pack into the context stages in middleware
|
||
testsPerOutCome = tests.GroupBy(t => t.TestOutcome).ToDictionary(t => t.Key, t => t.ToList()); | ||
|
||
foreach (var outcome in Enum.GetValues(typeof(TestOutcome)).OfType<TestOutcome>()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can simplify this by doing an earlier init. Move it up and do a select to dictionary on all enum values. Then we can gurantee that the dictionary is initialized correctly.
} | ||
} | ||
|
||
result = new Dictionary<TestOutcome, TestResult>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That also makes this into a simple linq to dictionary
private readonly Dictionary<TestOutcome, List<TestCase>> testsPerOutCome; | ||
private readonly Dictionary<TestOutcome, TestResult> result; | ||
|
||
public TestSummary(IEnumerable<TestCase> tests) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not 100% happy with putting so much logic into the ctor
, we should move that to a static factory method and do a normal value assign public ctor
|
||
public int Count => tests.Count; | ||
|
||
public TestOutcome OutCome { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OutCome->Outcome
if (failedCount > 0) { Console.WriteLine($"\t{ColorScheme.Default.ErrorIcon} {failedCount} failed"); } | ||
|
||
Console.ForegroundColor = ColorScheme.Default.SuccessColor; | ||
if (tests.Count() == successCount) { Console.WriteLine($"\t{ColorScheme.Default.SuccessIcon} All tests passed"); } | ||
if (summary.Count() == successCount) { Console.WriteLine($"\t{ColorScheme.Default.SuccessIcon} All tests passed"); } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Total count was counted at line 80. Maybe save and reuse?
{ | ||
Console.WriteLine(); | ||
Console.ForegroundColor = ColorScheme.Default.DefaultColor; | ||
Console.WriteLine($"\t{tests.Count()} total ({(int)Math.Round(TimeSpan.FromTicks(tests.Sum(t => t.Duration.Ticks)).TotalSeconds, 0, MidpointRounding.AwayFromZero)} s)"); | ||
Console.WriteLine($"\t{summary.Count()} total ({(int)Math.Round(TimeSpan.FromTicks(summary.Sum(t => t.Duration.Ticks)).TotalSeconds, 0, MidpointRounding.AwayFromZero)} s)"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@biohazard999 Thoughts on potentially supporting pretty timespans for summary output? For example here you count seconds, but what if all results are a fraction. It could output XX ms or XX ns depending on the lowest value or some preset smallest unit. Something like this could help: https://humanizr.net/#humanize-timespan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I know humanize, the problem is, they don't support short notation yet. So we have to write it our selfs.
Diverged to much from master, needs rework |
Fixes #51