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.

Sign up for GitHub

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

Jump to bottom

Add class/assembly cleanup/init messages to first/last test #1390

Merged
merged 16 commits into from
Nov 24, 2022
Merged

Add class/assembly cleanup/init messages to first/last test #1390

merged 16 commits into from
Nov 24, 2022

Conversation

engyebrahim
Copy link
Member

fix: #1340

Evangelink
Evangelink reviewed Nov 16, 2022
View reviewed changes
src/Adapter/MSTest.TestAdapter/Execution/UnitTestRunner.cs Outdated Show resolved Hide resolved
src/Adapter/MSTest.TestAdapter/Execution/UnitTestRunner.cs Outdated Show resolved Hide resolved
src/Adapter/MSTest.TestAdapter/Execution/UnitTestRunner.cs Outdated Show resolved Hide resolved
src/Adapter/MSTest.TestAdapter/Execution/UnitTestRunner.cs Outdated Show resolved Hide resolved
src/Adapter/MSTest.TestAdapter/Execution/UnitTestRunner.cs Outdated Show resolved Hide resolved
src/Adapter/MSTest.TestAdapter/Execution/UnitTestRunner.cs Outdated Show resolved Hide resolved
test/E2ETests/Smoke.E2E.Tests/SuiteLifeCycleTests.cs Outdated Show resolved Hide resolved
test/UnitTests/MSTestAdapter.UnitTests/Execution/TestExecutionManagerTests.cs Show resolved Hide resolved
Evangelink
Evangelink reviewed Nov 23, 2022
View reviewed changes
Copy link
Member

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Could you please review the test caseInheritClassWithCleanupInheritanceBehaviorNone_ParentTestMethod where the order of calls changed?

Evangelink
Evangelink requested changes Nov 23, 2022
View reviewed changes
Copy link
Member

@Evangelink Evangelink left a comment

Choose a reason for hiding this comment

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

I think there is something fishy with the implementation (maybe how you find the test to attach cleanup results to) because it seems that the ClassCleanup.InheritanceBehaviorBeforeEachDerivedClass messages are not attached to the correct test.

Evangelink
Evangelink reviewed Nov 23, 2022
View reviewed changes
src/Adapter/MSTest.TestAdapter/ObjectModel/UnitTestResult.cs
@@ -183,14 +183,14 @@ internal TestResult ToTestResult(TestCase testCase, DateTimeOffset startTime, Da

if (!StringEx.IsNullOrEmpty(DebugTrace))
{
string debugTraceMessagesInStdOut = string.Format(CultureInfo.InvariantCulture, "\n\n{0}\n{1}", Resource.DebugTraceBanner, DebugTrace);
string debugTraceMessagesInStdOut = string.Format(CultureInfo.CurrentCulture, "{2}{2}{0}{2}{1}", Resource.DebugTraceBanner, DebugTrace, Environment.NewLine);
Copy link
Member

Choose a reason for hiding this comment

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

Use OS line return to avoid mixing line return kinds.

@Evangelink Evangelink self-requested a review November 23, 2022 22:18
Evangelink
Evangelink previously approved these changes Nov 23, 2022
View reviewed changes
@Evangelink Evangelink merged commit 31f86eb into microsoft:main Nov 24, 2022
@cremor cremor mentioned this pull request Jan 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Evangelink Evangelink Evangelink approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Assembly and class cleanup and init TestContext.WriteLine() don't appear in any place.
2 participants
@engyebrahim @Evangelink