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

Are we not supposed to create subclasses of TestMethodAttribute anymore? ("If using extension of TestMethodAttribute then please contact vendor") #4616

Open
marnilss opened this issue Jan 10, 2025 · 6 comments · May be fixed by #4641
Assignees
Labels

Comments

@marnilss
Copy link

I created a subclass of the TestMethodAttribute, and while investigating an exception thrown by a test, the error message was prefixed with the following text:

Exception thrown while executing test. If using extension of TestMethodAttribute then please contact vendor.

My need for this subclass, is to log each execution of a test to Application Insights, so that we can correlate the test result with the tested HTTP APIs (which also logs to Application Insights).

Basically:

[AttributeUsage(AttributeTargets.Method)]
public class LoggingTestMethodAttribute : Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute
{
    public override TestResult[] Execute(ITestMethod testMethod)
    {
        using var requestLogger = new RequestLogger(GlobalTestSetup.Instance.TelemetryClient, testMethod);
        requestLogger.Start();

        var result = base.Execute(testMethod);

        requestLogger.Stop(result);
        return result;
    }
}

I couldn't find any other way to do it, except with a subclass of TestMethodAttribute.
If we shouldn't be subclassing TestMethodAttribute, what are we supposed to do instead? Is there already a better way of doing this, or should we just ignore the ominous text/warning?

I guess this has something to do with the breaking changes you are planning for V4 ?

Best regards,
Marcus Nilsson

@Youssef1313
Copy link
Member

Youssef1313 commented Jan 10, 2025

@marnilss Thank you for reporting this.

I think the message should have had more info about an exception. Is it something you can see?

Also to be sure, are you hitting breakpoint at the beginning of Execute? Can you wrap the Execute logic in try/catch and see if you hit the catch and maybe there are more info about an exception here?

If you have a repro that you could share, that also will be appreciated.

@marnilss
Copy link
Author

I think the message should have had more info about an exception. Is it something you can see?

Yes, my real exception messages was prefixed with this warning. So I have been able to solve my problem causing my tests to fail. It seems that exceptions of type System.TypeInitializationException will be prefixed with this warning.

BUT I'm wondering about why you are adding this warning; "Exception thrown while executing test. If using extension of TestMethodAttribute then please contact vendor."

Will my subclass of TestMethodAttribute stop working in the near future?

@Youssef1313
Copy link
Member

The message basically tells you that your override of TestMethodAttribute.Execute is throwing an exception, while it shouldn't. Why do you get the impression that subclasses of TestMethodAttribute shouldn't be used anymore?

@marnilss
Copy link
Author

The message basically tells you that your override of TestMethodAttribute.Execute is throwing an exception, while it shouldn't. Why do you get the impression that subclasses of TestMethodAttribute shouldn't be used anymore?

I just thought it was very strange for it to appear, so perhaps I got a little paranoid.
Since it isn't a commonly used pattern to "warn" about subclassed implementations, I interpreted as this: you (authors of MSTest) wanted to be notified if anyone is subclassing TestMethodAttribute. Perhaps in preparation for sealing that attribute class in v4 or something similar.

@Evangelink
Copy link
Member

We definitely need to reword the exception message. We will do it for 3.8

@marnilss
Copy link
Author

Ok, so if there are no such plans (to deprecate or seal the TestMethodAttribute class), then I'm satisfied 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants