microsoft / testfx

MSTest framework and adapter
MIT License
714 stars 253 forks source link

Class Initialize inheritance #143

Closed pvlakshm closed 1 year ago

pvlakshm commented 7 years ago

Description

Allow a ClassInitialize in a base class of a TestClass so that in addition to initializing static variables in the test class, I can initialize static variables in the test base class.

UV item: https://visualstudio.uservoice.com/forums/121579-visual-studio/suggestions/2311366-support-classinitialize-in-base-class

MikeChristensen commented 7 years ago

I believe this works if the test class and base class are in the same assemblies. I'm curious if issue #23 fixed this behavior.

AbhitejJohn commented 7 years ago

This works for TestInitialize today. ClassInitialize however is not inherited from base classes. We only execute the ClassInitialize of that specific class. The fix for this should just be plugging in similar functionality for Class Initialize/Cleanup as we have for Test Initialize/Cleanup here. FWIW, this should be the default workflow. However, since this would be a change in behavior from v1 mstest, we would also want to add a setting to turn this off as appropriate.

MikeChristensen commented 7 years ago

That sounds great! Though it would be fairly easy to work-around for now, since your test class [ClassInitialize] static method could just manually call your base class [ClassInitialize] static method. Using a static constructor would also be a work around. Would love to see this "bug" fixed though! I've been working on a lot of unit tests lately that could really use this pattern.

TheXby commented 7 years ago

Hello Its also happened for "AssemblyInitialize" attribute and mentioned problem not happened for all Unit testing frameworks (nUnit, xUnit, MbUnit....) I will be very grateful if it will be fixed soon. Thanks.

arnonax commented 7 years ago

I'm very much pro this idea! However, there are some details that must be flushed out first: First, there's the backward compatibility issue that @AbhitejJohn mentioned. But in addition, I can think of at least 2 possible outcomes of inheriting ClassInitialize:

  1. ClassInitialize will be called only once before any derived class
  2. ClassInitialize will be called before each of the derived classes. I believe that each of these outcomes may be desired in different situations.

Therefore I suggest to add an optional enum parameter to the ClassInitializeAttribute with the following possible values:

Please let me know what you think.

AbhitejJohn commented 7 years ago

Thanks for detailing that out @arnonax . That does sound interesting although I was hoping to just stick with [ClassInitialize(InheritanceBehavior=ClassInitializeInheritance.BeforeEachDerivedClass)] that is inline with C# behavior and [ClassInitialize] or [ClassInitialize(InheritanceBehavior=ClassInitializeInheritance.None)] for compat. This does sound like a viable alternative to a runsettings switch that is more explicit in code. /cc: @pvlakshm and @harshjain2 to get their thoughts.

josephliccini commented 6 years ago

Could this work for [ClassCleanup] too?

arnonax commented 6 years ago

Of course. ClassCleanup should be symmetrical to ClassIntialize. However, please also see my thoughts about a better cleanup mechanism, as I descried in a comment from Sep 8 on Issue 250

JoeShmoel commented 6 years ago

Any news?

parrainc commented 5 years ago

Not sure if there's anyone working on this issue, but I've assumed that no one is working on it and grab this one.

I've started to implement this new enhancement, and so far so good. Added the parameter to the ClassInitialize attribute, created some unit tests, and it seems to be working fine with "ClassInitializeInheritance.None" and "ClassInitializeInheritance.BeforeEach". @AbhitejJohn if we will just stick with those two options, according to your comment, I think that by tomorrow I'd be creating the PR for your reviews. Otherwise, I'll keep working on the "ClassInitializeInheritance.OnceBeforeAnyDerivedClasses" as well, to get the three option for the ClassInitialize attr.

let me know your thoughts.

mprice-hw commented 5 years ago

@parrainc This is great news!!!! One question, will ClassCleanup also work in the same fashion?

parrainc commented 5 years ago

@mprice-hw that is correct. ClassCleanup should be symmetrical to ClassInitialize. The below output is from a test project I created with a preview package generated with my changes (which i'm still testing):

===> Init from GrandPa ==> Init from Parent => Init from Child1 ==> Init from GrandPa ==> Init from Parent => Init from Child2 => Cleaning up from Child1 ==> Cleaning up from Parent ===> Cleaning up from GrandPa => Cleaning up from Child2 ==> Cleaning up from Parent ===> Cleaning up from GrandPa

Notice that Init method from Parent test class has ClassInitializeInheritance.BeforeEachDerivedClass, while the init method from GrandPa test class has ClassInitializeInheritance.OnceBeforeAnyDerivedClasses. As of now, both are available (BeforeEach and BeforeAny) plus the default one (None), but just None and BeforeEach are working properly (as you can see in the output. Even though the GranPa init has BeforeAny defined, still behaving as BeforeEach --still checking this). Child1 and Child2 are init methods from two different test classes.

Anyone please, feel free to review/clone/fork my repo parrainc/testfx if you want to try this out, or maybe help me with anything i've missed.

@AbhitejJohn please advice if, in case this changes are approved, we'll be sticking to BeforeEach and None options, or also including the BeforeAny option. That way, I could focus more on polishing the first two, instead of the three ones (in case of not being necessary).

AceHack commented 5 years ago

Any update? Also does ClassInitialize and ClassCleanup work with Async i.e. Task results? Thanks.

AbhitejJohn commented 5 years ago

@parrainc : That sounds fantastic, I'm super late to this though but I think we could definitely start with just that two. I've moved on to other things but @jayaranigarg and @singhsarab could help if you still need any.

@AceHack : From here ClassInitialize and ClassCleanup should be async Task friendly too.

parrainc commented 5 years ago

@AbhitejJohn cool, It's been a while since I opened the testfx project on my computer, so I have to get my fork up-to-date. After that, will be submitting the PR so @jayaranigarg and @singhsarab can take a look when available.

jayaranigarg commented 5 years ago

Consume this change using following nuget packet versions: https://dotnet.myget.org/feed/mstestv2/package/nuget/MSTest.TestAdapter/2.0.0-build-20190708-01 https://dotnet.myget.org/feed/mstestv2/package/nuget/MSTest.TestFramework/2.0.0-build-20190708-01

mills-andrew commented 5 years ago

This issue is still occuring for me, which is very weird because yesterday it seemed just fine.

`

namespace Tests
{
    [TestClass]
    public abstract class BaseTest
    {
        protected static TestContext mContext;

        [AssemblyInitialize]
    public static void Assembly(TestContext pContext)
    {
        Core.Construct<FrameworkConstruction>()
            .AddProperties(pContext.Properties)
            .AddConsoleLogger().AddDebugLogger().AddFileLogger()
            .Build();

        Core.Log.Information("AssemblyInitialize: " + System.Threading.Thread.CurrentThread.ManagedThreadId.ToString());
    }

    [AssemblyCleanup]
    public static void AssemblyCleanup()
    {
        Core.Log.Information("AssemblyCleanup: " + System.Threading.Thread.CurrentThread.ManagedThreadId.ToString());
    }

    [ClassInitialize]
    public static void ClassInitialize(TestContext pContext)
    {
        mContext = pContext;
        Core.Log.Debug("ClassInitialize: " + System.Threading.Thread.CurrentThread.ManagedThreadId.ToString());
    }

    [ClassCleanup]
    public static void ClassCleanup()
    {
        Core.Log.Debug("ClassCleanUp: " + System.Threading.Thread.CurrentThread.ManagedThreadId.ToString());
    }
}

} ` Above you can see my base test. below you can see my unit tests

namespace Tests { [TestClass] public class UnitTest1 : BaseTest { [TestMethod] public void TestMethod1() { Core.Log.Debug("TestMethod: " + System.Threading.Thread.CurrentThread.ManagedThreadId.ToString()); } } }

The assembly is being called, the test method is being called.

INFORMATION: [2019-08-09 08:57:23] AssemblyInitialize: 12 [BaseTest.cs > Assembly() > Line 21] INFORMATION: [2019-08-09 08:57:23] TestMethod: 12 [UnitTest1.cs > TestMethod1() > Line 15] INFORMATION: [2019-08-09 08:57:23] AssemblyCleanup: 4 [BaseTest.cs > AssemblyCleanup() > Line 27]

ClassInitialize and Cleanup are not being called

AbhitejJohn commented 5 years ago

@KitoCoding : For compat reasons this is an opt in at the moment. You'd need to add [ClassInitialize(InheritanceBehavior.BeforeEachDerivedClass)] and [ClassCleanup(InheritanceBehavior.BeforeEachDerivedClass)] to your base class if you want them to be invoked. @jayaranigarg : Do we also plan to add a runsettings configuration to tune this globally?

mills-andrew commented 5 years ago

@AbhiteJohn I don't see this feature you speak of. Am i missing a namespace I am not familiar with?

AbhitejJohn commented 5 years ago

@KitoCoding : you'd need the packages from the daily feed Jaya posted earlier. It doesn't look like this is part of the public nuget feed just yet.

mills-andrew commented 5 years ago

@AbhiteJohn, this Nuget package is missing. Install-Package MSTest.TestFramework -Version 2.0.0 -Source https://dotnet.myget.org/F/mstestv2/api/v3/index.json

I have tried with the different builds, but it is not able to find any.

parrainc commented 5 years ago

@KitoCoding I was able to get the changes from version 2.0.0 in the daily feed. And your example code worked with no issues.

NightOwl888 commented 5 years ago

Not sure if this was a design choice or a bug, but for this simple scenario, InitMe is called twice. I would expect only one call here because there is technically only one executable test class.

Two calls is better than none, but it does complicate things if we have to track these extra calls in order to prevent test data from cleaning up before all of the tests are done.

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace SomeTests
{
    [TestClass()]
    public abstract class TestCase
    {
        [ClassInitialize(InheritanceBehavior.BeforeEachDerivedClass)]
        public static void InitMe(TestContext context)
        {

        }

        [TestMethod]
        public void TestFromBase()
        {

        }

        [ClassCleanup(InheritanceBehavior.BeforeEachDerivedClass)]
        public static void CleanMe()
        {

        }
    }

    [TestClass()]
    public class DerivedTestCase : TestCase
    {
        [TestMethod]
        public void TestFromDerived()
        {

        }
    }
}

In addition, CleanMe is not getting called at all. That definitely seems like a bug.

NightOwl888 commented 5 years ago

I found another issue: When the derived type is in another assembly that is referenced in the same project, the FullyQualifiedTestClassName doesn't include the name of the other assembly. So, when calling Type.GetType(context.FullyQualifiedTestClassName), it resolves to null.

Since I am developing a test case framework for an application for 3rd parties to inherit, I have no way of knowing what the assembly name might be, and it wouldn't be very efficient for me to have to load all referenced assemblies to find it.

Also, since my goal is to look for one of my custom attributes on the 3rd party test class, it seems like it might be a good idea to add the TestClassType as a property so I don't have to gamble with the string actually resolving to something.

mbranscomb commented 5 years ago

@NightOwl888 Thanks for the alert to ClassInitialize running multiple times - saved me some time trouble-shooting.

For now I'm keeping track of the current test class on the TestContext which seems sufficient for my needs.

[ClassInitialize(InheritanceBehavior.BeforeEachDerivedClass)]
public static void ClassInitialize(TestContext testContext)
{
    try
    {
        // Due to an apparent bug ClassInitialize is called twice (or perhaps multiple times).
        // See https://github.com/Microsoft/testfx/issues/143
        if (testContext.FullyQualifiedTestClassName == _currentTestClass)
        {
            return;
        }
        _currentTestClass = testContext.FullyQualifiedTestClassName;
        ...            
mills-andrew commented 5 years ago

It should also be noted that the class initialization attribute is not being run on the same thread as the test attributes. The test attributes when a worker is running off of the level of test methods should run on separate threads as the class initialization attributes however when a worker is running on a class level then they should all run on the same thread.

NightOwl888 commented 5 years ago

@KitoCoding

Thanks. That is a huge problem for me because my test fixture sets the culture during initialization of the class, which obviously needs to be on the same thread as the test in order to function.

mills-andrew commented 5 years ago

@KitoCoding Thanks. That is a huge problem for me because my test fixture sets the culture during initialization of the class, which obviously needs to be on the same thread as the test in order to function.

I have brought this attention to the team, unfortunately they see this as a low priority. To me this breaks the feature of parallel testing on a classLevel. I have a threadManager that handles all of my objects on a per worker base. Since they offer no way of handling unique IDs per worker, I have to rely on ManagedThreadIds. But since the Init Class attribute and clean up runs on separate threads, I can't clean up worker nodes on a class level.

konradperzyna commented 4 years ago

Has anyone found a workaround for [ClassCleanup] not called at all? This is a great feature, but useless as it is implemented for now. I also guess that ClassCleanup should be called AFTER every derived class cleanup to keep the symetry (so ClassCleanup(InheritanceBehavior.BeforeEachDerivedClass) is at least misleading).

mills-andrew commented 4 years ago

Class cleanup as of the latest stable build will not run if the class clean up is not inside the class you are calling from. This means you can't put it in your base test. A simple workaround would be to create a function call that will trigger the initialize or cleanup method. But regarding your other issue 4test class clean up not being triggered after all of the tests have been executed. this is a known issue but the Microsoft team has already announced that this is not a priority to them. I suggest not relying on worker nodes to run on a class level rather to have the one on a method level. I 100% agree that this is silly and should be addressed but they have made it clear that their priorities are not to fix this

NGloreous commented 4 years ago

Added PR to fix issue where base class cleanup isn't called.

maboivin commented 4 years ago

Are there any plans to fix the ClassInitialize method being called multiple times for the same class?

From my (very) simple tests, like other have already mentioned, it looks like the ClassInitialize method is called once for each test. I used the following dependencies:

<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />
<PackageReference Include="MSTest.TestAdapter" Version="2.1.0" />
<PackageReference Include="MSTest.TestFramework" Version="2.1.0" />

TestBase.cs

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TestFxIssue143
{
    [TestClass]
    public class TestBase
    {
        public TestContext TestContext { get; set; }

        [ClassInitialize(InheritanceBehavior.BeforeEachDerivedClass)]
        public static void ClassInitialize(TestContext context)
        {
            Console.WriteLine($"[{DateTime.Now.ToString("HH:mm:ss.fff")}] ClassInitialize: {context.FullyQualifiedTestClassName}/{context.TestName}");
        }

        [ClassCleanup(InheritanceBehavior.BeforeEachDerivedClass)]
        public static void ClassCleanup()
        {
            Console.WriteLine($"[{DateTime.Now.ToString("HH:mm:ss.fff")}] ClassCleanup");
        }

        [TestInitialize()]
        public void TestInitialize()
        {
            Console.WriteLine($"[{DateTime.Now.ToString("HH:mm:ss.fff")}] TestIntialize: {this.TestContext.FullyQualifiedTestClassName}/{this.TestContext.TestName}");
        }

        [TestCleanup()]
        public void TestCleanup()
        {
            Console.WriteLine($"[{DateTime.Now.ToString("HH:mm:ss.fff")}] TestCleanup: {this.TestContext.FullyQualifiedTestClassName}/{this.TestContext.TestName}");
        }
    }
}

UnitTest1.cs

using System;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TestFxIssue143
{
    [TestClass]
    public class UnitTest1 : TestBase
    {
        [TestMethod]
        public void TestMethod1()
        {
            Console.WriteLine($"[{DateTime.Now.ToString("HH:mm:ss.fff")}] TestMethod: {this.TestContext.FullyQualifiedTestClassName}/{this.TestContext.TestName}");
        }

        [TestMethod]
        public void TestMethod2()
        {
            Console.WriteLine($"[{DateTime.Now.ToString("HH:mm:ss.fff")}] TestMethod: {this.TestContext.FullyQualifiedTestClassName}/{this.TestContext.TestName}");
        }

        [TestMethod]
        public void TestMethod3()
        {
            Console.WriteLine($"[{DateTime.Now.ToString("HH:mm:ss.fff")}] TestMethod: {this.TestContext.FullyQualifiedTestClassName}/{this.TestContext.TestName}");
        }
    }
}

And the output is: TestMethod1

[16:43:26.249] ClassInitialize: TestFxIssue143.UnitTest1/TestMethod1
[16:43:26.261] TestIntialize: TestFxIssue143.UnitTest1/TestMethod1
[16:43:26.261] TestMethod: TestFxIssue143.UnitTest1/TestMethod1
[16:43:26.262] TestCleanup: TestFxIssue143.UnitTest1/TestMethod1

TestMethod2:

[16:43:26.272] ClassInitialize: TestFxIssue143.UnitTest1/TestMethod2
[16:43:26.272] TestIntialize: TestFxIssue143.UnitTest1/TestMethod2
[16:43:26.273] TestMethod: TestFxIssue143.UnitTest1/TestMethod2
[16:43:26.273] TestCleanup: TestFxIssue143.UnitTest1/TestMethod2

TestMethod3:

[16:43:26.273] ClassInitialize: TestFxIssue143.UnitTest1/TestMethod3
[16:43:26.273] TestIntialize: TestFxIssue143.UnitTest1/TestMethod3
[16:43:26.273] TestMethod: TestFxIssue143.UnitTest1/TestMethod3
[16:43:26.273] TestCleanup: TestFxIssue143.UnitTest1/TestMethod3

I noticed the ClassCleanup is not written in the output but it did get called only once which is the expected behavior, but the ClassInitialize is called 3 times and I would have expected only 1 time since there's only 1 class.

nohwnd commented 4 years ago

@maboivin there are not plans to fix this at the moment, for the next version we are focusing on stabilization, performance and low-hanging fruit, which this unfortunately is not, but this issue has multiple upvotes and we go from the most upvoted / most commented when looking at issues, so hopefully soon.

NGloreous commented 4 years ago

@nohwnd consider #705 to fix the duplicate class init bug. @maboivin FYI.

maboivin commented 4 years ago

Good news. Thanks. I will take a look when the next release is available.

poserdonut commented 3 years ago

I have the following in my base class:

        [ClassCleanup(InheritanceBehavior.BeforeEachDerivedClass)]
        public static void ClassCleanup()
        {
            Console.WriteLine($"[{DateTime.Now.ToString("HH:mm:ss.fff")}] ClassCleanup");
        }

And it's not being executed for the classes inherting from the base class. Shouldn't it work?

NGloreous commented 3 years ago

@poserdonut yes that should work. Are you using the latest version that has these changes? How did you confirm it didn't get executed? Did you try in a debugger? Note that your text in your call to WriteLine won't end up getting logged anywhere.

poserdonut commented 3 years ago

Latest version (not preview) and we were using debug. Had to switch to testcleanup instead.

enisak commented 2 years ago

Latest version (not preview) and we were using debug. Had to switch to testcleanup instead.

Same here :(

enisak commented 2 years ago

@poserdonut yes that should work. Are you using the latest version that has these changes? How did you confirm it didn't get executed? Did you try in a debugger? Note that your text in your call to WriteLine won't end up getting logged anywhere.

Any luck so far? This is really not working as it should

DimoYa commented 1 year ago

any update ?

Evangelink commented 1 year ago

I am confused as there seems to be two different "things" (one feature request and one bug) being discussed here.

As far as I can see, the inheritance of the class initialize is implemented and is working properly (we recently added a bunch of integration tests for this: https://github.com/microsoft/testfx/blob/2d2ab810a78fa98855a9e56cb5a18d7f963c45f2/test/IntegrationTests/MSTest.VstestConsoleWrapper.IntegrationTests/SuiteLifeCycleTests.cs).

@enisak Could you please let us know which version of MSTest you are using? Also, would you mind providing a small repro of what's not working so we can investigate (cc @engyebrahim)?

NGloreous commented 1 year ago

The feature request was completed. There were some bugs with it that I fixed but AFIK there aren't any other issues. The feature is working fine for my team. Maybe this should get closed out an bugs be opened if anyone has specific issues.

Evangelink commented 1 year ago

Agreed, let me do that now.