Skip to content
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

Fix multiple results not returned with custom TestMethod and TestClass (#358) #363

Merged
merged 9 commits into from
Mar 28, 2018
Merged

Fix multiple results not returned with custom TestMethod and TestClass (#358) #363

merged 9 commits into from
Mar 28, 2018

Conversation

bignoncedric
Copy link
Contributor

This PR fixes the bug #358.
In TestMethodRunner, only the first element of TestResult[] returned by Executor.Execute was used.

A test is also added to validate the RFC 003- Framework Extensibility for Custom Test Execution.

@msftclas
Copy link

msftclas commented Feb 6, 2018

CLA assistant check
All CLA requirements met.

currentResult.Duration = watch.Elapsed;
foreach (var testResult in testResults)
{
testResult.DatarowIndex = rowIndex++;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DatarowIndex is used for each row of data in dataSource.
By framework extensibility you have now started running each dataRow multiple times(say 5), but we should not increase dataRowIndex for each of the 5 runs of a DataRow. It should be incremented only once per dataRow and not once per testResult.
This statement should be testResult.DataRowIndex = rowIndex;
Do rowIndex++ separately outside of the loop.

@@ -160,6 +160,7 @@ public void ValidatePassedTestsContain(params string[] passedTests)
{
foreach (var test in passedTests)
{
var x = string.Join(", ", this.runEventsHandler.PassedTests.Select(_ => _.DisplayName));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this.

Assert.AreNotEqual(3, customTestClass1ExecutionCount);
}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a data-driven test with [IterativeTestMethod] attribute

@jayaranigarg
Copy link
Member

Kindly sign the CLA agreement as well.

@jayaranigarg
Copy link
Member

Thank you for your contribution @bignoncedric . This PR looks awesome.
As per our offline conversation, I am keeping this PR on hold. Let me know whenever you are ready to push this in.

@jayaranigarg
Copy link
Member

@bignoncedric : Any updates here?

@federicoce-moravia
Copy link

Any updates? I'm currently sorting the ITestResult[] with a "worst outcome first" comparer before returning, but I would really like to see this handled elegantly by the library.

@ShreyasRmsft
Copy link
Member

@bignoncedric any updates?

@jayaranigarg
Copy link
Member

@dotnet-bot Test Windows / Debug Build please

@jayaranigarg jayaranigarg merged commit 3e4ca97 into microsoft:master Mar 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants