testfx icon indicating copy to clipboard operation
testfx copied to clipboard

Class Initialize inheritance

Open pvlakshm opened this issue 7 years ago • 40 comments

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

pvlakshm avatar Apr 07 '17 09:04 pvlakshm

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.

MikeChristensen avatar Apr 19 '17 17:04 MikeChristensen

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.

AbhitejJohn avatar Apr 19 '17 18:04 AbhitejJohn

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.

MikeChristensen avatar Apr 19 '17 18:04 MikeChristensen

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.

TheXby avatar May 24 '17 10:05 TheXby

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:

  • [ClassInitialize] or [ClassInitialize(InheritanceBehavior=ClassInitializeInheritance.None)] - ClassInitialize is called only for tests defined in the same class, as in V1.
  • [ClassInitialize(InheritanceBehavior=ClassInitializeInheritance.OnceBeforeAnyDerivedClasses)] - ClassInitialize will be called only once before any derived class
  • [ClassInitialize(InheritanceBehavior=ClassInitializeInheritance.BeforeEachDerivedClass)] - ClassInitialize will be called before each derived class.

Please let me know what you think.

arnonax avatar Aug 25 '17 14:08 arnonax

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.

AbhitejJohn avatar Aug 28 '17 05:08 AbhitejJohn

Could this work for [ClassCleanup] too?

josephliccini avatar Oct 19 '17 18:10 josephliccini

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

arnonax avatar Oct 19 '17 19:10 arnonax

Any news?

JoeShmoel avatar Jun 01 '18 12:06 JoeShmoel

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.

parrainc avatar Oct 17 '18 21:10 parrainc

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

mprice-hw avatar Nov 04 '18 00:11 mprice-hw

@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).

parrainc avatar Nov 07 '18 22:11 parrainc

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

AceHack avatar Mar 04 '19 04:03 AceHack

@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.

AbhitejJohn avatar Mar 05 '19 06:03 AbhitejJohn

@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.

parrainc avatar Mar 05 '19 21:03 parrainc

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

jayaranigarg avatar Jul 08 '19 10:07 jayaranigarg

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

mills-andrew avatar Aug 09 '19 12:08 mills-andrew

@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?

AbhitejJohn avatar Aug 09 '19 18:08 AbhitejJohn

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

mills-andrew avatar Aug 09 '19 18:08 mills-andrew

@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.

AbhitejJohn avatar Aug 09 '19 19:08 AbhitejJohn

@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.

mills-andrew avatar Aug 13 '19 11:08 mills-andrew

@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.

parrainc avatar Aug 13 '19 16:08 parrainc

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 avatar Aug 23 '19 13:08 NightOwl888

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.

NightOwl888 avatar Aug 23 '19 14:08 NightOwl888

@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;
        ...            

mbranscomb avatar Sep 10 '19 16:09 mbranscomb

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.

mills-andrew avatar Sep 10 '19 16:09 mills-andrew

@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.

NightOwl888 avatar Sep 11 '19 00:09 NightOwl888

@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.

mills-andrew avatar Sep 11 '19 01:09 mills-andrew

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).

konradperzyna avatar Sep 18 '19 09:09 konradperzyna

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

mills-andrew avatar Sep 18 '19 10:09 mills-andrew