Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 28, 2026

Plan to fix Assert.Inconclusive behavior in constructor

  • Understand the issue: Assert.Inconclusive in ctor results in test failure vs TestInitialize which marks as skipped
  • Analyze code: Found that TestMethodInfo.cs handles TestInitialize correctly but not constructor
  • Simplify fix: Remove exception handling from CreateTestClassInstance and let caller handle all exceptions
  • Add unit test: Create test for constructor throwing AssertInconclusiveException
  • Update unit test: Match expected error message with new exception handling approach
  • Add acceptance test: Extend existing InconclusiveTests to cover constructor scenario
  • Fix enum order: Constructor lifecycle phase should come after ClassInitialize, not before
  • Fix XML documentation: Remove obsolete parameter documentation
  • Use expression body: Convert CreateTestClassInstance to expression body syntax
  • Security scan: Run CodeQL checker - no issues found
  • Build and test: Validate changes work correctly

Changes

Simplified the fix by removing the catch block from CreateTestClassInstance entirely. The caller's exception handler already checks for AssertInconclusiveException and sets the outcome correctly (lines 479-480). This approach:

  • Eliminates code duplication
  • Allows the outer handler to manage all exceptions consistently
  • Still correctly handles Assert.Inconclusive in constructors
  • Uses the assert exception's own message instead of wrapping it
Original prompt

This section details on the original issue you should resolve

<issue_title>Assert.Inconclusive behavior is different in ctor vs TestInitialize</issue_title>
<issue_description>## Describe the bug

Doing a call to Assert.Inconclusive in ctor results in test failure while the same in a TestInitialize method results in test cosndidered as skipped. I think the behavior should be the same across the 2 sides.

Steps To Reproduce

[TestClass]
public sealed class MyTestClass
{
    public MyTestClass()
    {
        Assert.Inconclusive("Skipped"); // TestMethod is considered as failing
    }

    [TestMethod]
    public void TestMethod()
    {         
    } 
}

while

[TestClass]
public sealed class MyTestClass
{
    [TestInitialize]
    public void TestInit()
    {
        Assert.Inconclusive("Skipped"); // TestMethod is considered as skipped
    }

    [TestMethod]
    public void TestMethod()
    {         
    } 
}

My current actual scenario is to skip based on TFM.</issue_description>

<agent_instructions>Handling around ctor invocation of test classes should follow similar logic as calls to the TestInitialize. Change should be tested with unit tests and acceptance test</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Fix Assert.Inconclusive behavior between ctor and TestInitialize Fix Assert.Inconclusive in constructor to behave consistently with TestInitialize Jan 28, 2026
Copilot AI requested a review from Evangelink January 28, 2026 16:19
result.Outcome = UTF.UnitTestOutcome.Timeout;
result.TestFailureException = new TestFailedException(UTFUnitTestOutcome.Timeout, string.Format(CultureInfo.CurrentCulture, Resource.Execution_Test_Timeout, TestMethodName, TimeoutInfo.Timeout));
}
else if (realException is AssertInconclusiveException)
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need all this here?
The caller of CreateTestClassInstance is already wrapped in try/catch. Should we let the exception bubble up from CreateTestClassInstance to be handled by the caller? Or is there really some special handling needed specific to CreateTestClassInstance?

Copy link
Member

Choose a reason for hiding this comment

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

I did some manual lookup and it's also unclear why we need this catch at all.

@copilot can you try removing the catch entirely and letting the caller handle the exception

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The exception handling in CreateTestClassInstance is necessary and follows the existing pattern in the codebase. If we let the exception bubble up:

  1. The error message would be incorrect - it would use UTA_TestMethodThrows instead of UTA_InstanceCreationError
  2. We'd need to re-throw after catching, complicating control flow
  3. The caller checks _classInstance != null to determine if execution should continue

The current pattern allows constructor failures to gracefully set the result and return null, so the caller can skip TestInitialize and remaining phases. This is consistent with how timeout exceptions in constructors are already handled (line 1036-1039).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 7f17965. Removed the entire catch block from CreateTestClassInstance. The method now simply invokes the constructor and returns the instance. Any exceptions (including AssertInconclusiveException) bubble up to the caller's exception handler which already has the logic to handle them correctly at lines 479-480.

@Evangelink
Copy link
Member

@copilot test fails with:

src/TestFramework/TestFramework/Assertions/Assert.Contains.cs(481,1): error : [TestOutcomeShouldBeRespectedCorrectly (ClassInitialize)] Assert.Contains failed. String 'MSTest v4.1.0-ci (UTC 01/28/2026) [linux-x64 - .NET 10.0.2]
AssemblyInitialize called
ClassInitialize called
ClassCleanup called
AssemblyCleanup called
skipped TestMethod (0ms)
  Class Initialization method UnitTest1.ClassInit threw exception. Microsoft.VisualStudio.TestTools.UnitTesting.AssertInconclusiveException: Assert.Inconclusive failed. .
Test run summary: Zero tests ran - /mnt/vss/_work/1/s/artifacts/tmp/Debug/testsuite/3AVvb/TestInconclusive/bin/Release/net10.0/TestInconclusive.dll (net10.0|x64)
  total: 1
  failed: 0
  succeeded: 0
  skipped: 1
  duration: 1s 038ms' does not contain string 'Constructor called'. 'substring' expression: 'value', 'value' expression: 'testHostResult.StandardOutput'. Expression 'AssertOutputContains' failed for member 'TestOutcomeShouldBeRespectedCorrectly' at line 60 of file '/_/test/IntegrationTests/MSTest.Acceptance.IntegrationTests/InconclusiveTests.cs'. Output of the test host is:
Command: /mnt/vss/_work/1/s/artifacts/tmp/Debug/testsuite/3AVvb/TestInconclusive/bin/Release/net10.0/TestInconclusive --settings my.runsettings
====================
ExitCode: 8
====================
StandardOutput:
MSTest v4.1.0-ci (UTC 01/28/2026) [linux-x64 - .NET 10.0.2]
AssemblyInitialize called
ClassInitialize called
ClassCleanup called
AssemblyCleanup called
skipped TestMethod (0ms)
  Class Initialization method UnitTest1.ClassInit threw exception. Microsoft.VisualStudio.TestTools.UnitTesting.AssertInconclusiveException: Assert.Inconclusive failed. .
Test run summary: Zero tests ran - /mnt/vss/_work/1/s/artifacts/tmp/Debug/testsuite/3AVvb/TestInconclusive/bin/Release/net10.0/TestInconclusive.dll (net10.0|x64)
  total: 1
  failed: 0
  succeeded: 0
  skipped: 1
  duration: 1s 038ms
====================
StandardError:

Copy link
Contributor Author

Copilot AI commented Jan 28, 2026

@copilot test fails with:

...

Fixed in 637c67a. The issue was that Constructor was placed at the beginning of the Lifecycle enum, but it should come after ClassInitialize in the actual execution order:

  1. AssemblyInitialize
  2. ClassInitialize
  3. Constructor ← moved here
  4. TestInitialize
  5. TestMethod
    ...

The enum ordering now matches the actual test lifecycle execution order.

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.

Assert.Inconclusive behavior is different in ctor vs TestInitialize

3 participants