-
Notifications
You must be signed in to change notification settings - Fork 256
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
Setting Option for marking not runnable tests as failed #524
Setting Option for marking not runnable tests as failed #524
Conversation
@@ -161,7 +161,7 @@ internal void SendTestResults(TestCase test, UnitTestResult[] unitTestResults, D | |||
continue; | |||
} | |||
|
|||
var testResult = unitTestResult.ToTestResult(test, startTime, endTime, MSTestSettings.CurrentSettings.MapInconclusiveToFailed); | |||
var testResult = unitTestResult.ToTestResult(test, startTime, endTime, MSTestSettings.CurrentSettings.MapInconclusiveToFailed, MSTestSettings.CurrentSettings.MapNotRunnableToFailed); |
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.
Instead of passing individual values from settings : MapInconclusiveToFailed and MapNotRunnableToFailed, prefer passing the entire settings. In future, we can extract whichever value will be required.
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.
Addressed. Passing entire adapter settings now. Modified the tests with these as well.
/// <returns>The Test platforms outcome.</returns> | ||
internal static TestOutcome ToTestOutcome(UnitTestOutcome unitTestOutcome, bool mapInconclusiveToFailed) | ||
internal static TestOutcome ToTestOutcome(UnitTestOutcome unitTestOutcome, bool mapInconclusiveToFailed, bool mapNotRunnableToFailed) |
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.
Same here. Pass entire settings.
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.
Addressed. Passing entire adapter settings now.
@@ -280,6 +287,7 @@ private static MSTestSettings ToSettings(XmlReader reader) | |||
// <MSTestV2> | |||
// <CaptureTraceOutput>true</CaptureTraceOutput> | |||
// <MapInconclusiveToFailed>false</MapInconclusiveToFailed> | |||
// <MapNotRunnableToFailed>false</MapNotRunnableToFailed> |
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.
Nice 👍
…ingNotRunnableToFailedOption
Tagging @pvlakshm to review this as well. |
@PBoraMSFT Can you please review this and see if it's good to check in? |
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.
What about the documentation ?
And should we make this discoverable, is this is marked false, probably a message like, "Unable to run some tests, please set "MapNotRunnableToFailed" in MSTest runsettings to mark these tests as failed." ?
Overall, changes look good.
@PBoraMSFT This would need to be updated in the documentation that |
@vagisha-nidhi @singhsarab - let's not accumulate doc debt on our repos. When raising PRs, please include doc changes as well - this has been pending since Dec. Can you please log a new GH issue for this and fix it? All testfx docs are right here in the repo. |
@PBoraMSFT I had left a comment for it. Not sure if we should be blocking the PRs for that (I vote for blocking the PR)
@vagisha-nidhi Can you please add the docs for this one please. |
Updating the docs via microsoft/testfx-docs#71 |
Issue related #499