microsoft
[MSTest][v4] STAThreadAttribute should be sealed
We should seal the attribute, and no longer make it special cased by the adapter. Instead, the attribute should override Execute (or ExecuteAsync) to invoke the test in an STA thread.
Will there still be a way to customize test methods that run in an STA thread? We find it useful to override this attribute to add additional functionality for STA tests
@RobSwDev If we took the breaking change, it should still be easy enough to create your custom STATestMethod, but inherit directly from TestMethodAttribute instead of STATestMethodAttribute. But I would like to understand more your scenario. What additional functionality you do add?
If your custom attribute overrides Execute method, do you add some logic before/after a base.Execute call? Does the extra logic require running on STA, or would you expect only base.Execute to run on STA? Or does it not matter at all for your scenario? Example:
public class DerivedSTATestMethodAttribute : STATestMethodAttribute
{
public override TestResult[] Execute(ITestMethod testMethod)
{
// logic before base.Execute.
// Do you expect this logic to be on STA? Or does it not matter for your scenario?
base.Execute(testMethod); // calls the test method, which will run on STA thread.
// logic after base.Execute.
// Do you expect this logic to be on STA? Or does it not matter for your scenario?
}
}
We have some tests that can only run in certain environments or configurations. E.g. WindowsOnly, LinuxOnly, DebugOnly, ReleaseOnly, GAOnly are universal, but we also have some bespoke switches for our particular app.
We decorate such tests with attributes, and our customizations of the TestMethod attributes (both plain and STA flavour) check these "Requirement" attributes and skips the test (by returning Inconclusive) if the requirements aren't met.
So for this particular example, I don't think our "SkipTest" logic has to run on an STA thread.
We've also put some "universal" cleanup logic in the test attribute, and I just noticed that such cleanup logic is no longer being run due to #5707. Annoyingly, it's the STA tests that need this cleanup most of all.
@RobSwDev We recently shipped ConditionBaseAttribute along with OSConditionAttribute. Does using these attribute help with proper conditioning and not needing to inherit from STAThreadAttribute for your case?
Yes - ConditionBaseAttribute definitely looks helpful - we will switch to those, and get rid of our bespoke implementation.
Looking at the class though, it's a little confusing. There's an abstract ShouldRun property, but also a ConditionMode. If Mode is ConditionMode.Exclude, does that mean the test will run if ShouldRun returns false?
Yes. If Mode is Exclude, it will run when ShouldRun returns false.
The implementation of ShouldRun should be done with "include" mode in mind.
If you are curious about the actual logic behind it, it's here.
One other thing we do before & after the test is attach and detach from Application.ThreadException
(Whether we need to do this or not is not clear to me!)
If this action is taken in TestInitialize/TestCleaup, then cleanup sometimes runs on a different thread, leading to a memory leak.
But doing it in the overridden STATestMethodAttribute.Execute does not seem to leak.
Thanks for your input @RobSwDev. We may end up not sealing the attribute, but slightly have a break in behavior in v4. We will likely make it such that Execute override is run on non-STA, but the actual method is STA. We can still be able to add an ExecuteInner or ExecuteCore method which is run in STA.
Hi @Youssef1313 We're changing to use ConditionBaseAttribute. One of the conditions we want to create depends on the TestContext. What's the best way of achieving this? Something like reading and setting a static value in an AssemblyInitialize method? Thanks Rob
ConditionBaseAttribute unfortunately cannot access TestContext.
@nohwnd Do you think it's worth breaking the public API for v4 to support this case?
Yes, it seems that the ConditionBaseAttributes are evaluated just before any AssemblyInitialize is run. But the TestContext has already been created - it just isn't visible to the ConditionBaseAttributes.
The test Settings dictionary (which is all I care about) could be exposed via a public static property - similar to MSTestSettings.CurrentSettings, which sadly does not include any of the TestParameters. Although I see that class is marked as Obsolete..
Is there anywhere else I could hook into to obtain the current settings dictionary before the ConditionBaseAttribute.ShouldRun is evaluated?
Unfortunately, TestContext isn't available to ConditionBaseAttribute :(