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

Class Initialize inheritance #143

Closed
pvlakshm opened this issue Apr 7, 2017 · 44 comments
Closed

Class Initialize inheritance #143

pvlakshm opened this issue Apr 7, 2017 · 44 comments

Comments

@pvlakshm
Copy link
Contributor

pvlakshm commented Apr 7, 2017

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 pvlakshm mentioned this issue Apr 7, 2017
16 tasks
@MikeChristensen
Copy link

MikeChristensen commented Apr 19, 2017

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
Copy link
Contributor

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 AbhitejJohn added enhancement Help-Wanted The issue is up-for-grabs, and can be claimed by commenting labels Apr 19, 2017
@MikeChristensen
Copy link

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
Copy link

TheXby commented May 24, 2017

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
Copy link

arnonax commented Aug 25, 2017

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.

@AbhitejJohn
Copy link
Contributor

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
Copy link

Could this work for [ClassCleanup] too?

@arnonax
Copy link

arnonax commented Oct 19, 2017

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
Copy link

Any news?

@parrainc
Copy link
Contributor

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
Copy link

mprice-hw commented Nov 4, 2018

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

@parrainc
Copy link
Contributor

parrainc commented Nov 7, 2018

@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
Copy link

AceHack commented Mar 4, 2019

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

@AbhitejJohn
Copy link
Contributor

@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
Copy link
Contributor

parrainc commented Mar 5, 2019

@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 pushed a commit that referenced this issue Jul 8, 2019
* Implemented Initialize Inheritance for ClassInitialize attribute

* Implemented Cleanup inheritance behavior for ClassCleanup attr

* Removed OnceBeforeAnyDerivedClasses from ClassInitializeInheritance enum

* Changed UTF.TestClass for custom testclass attr to avoid test ouput warnings

* Updated RunClassCleanup to prevent running base cleanup without having run base init

* Fix name typo

* added doc for BaseClassInitializeMethodsDict prop

* Updates per review comments

* making inheritance behavior enum more generic and updating tests

* done some refactoring and added/fixed a few tests

* updates as per review comments

* updated classinfo and tests

* updated classcleanup 1-param ctor

* updating doc message for 1-param ctors
@mills-andrew
Copy link

mills-andrew commented Aug 9, 2019

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
Copy link
Contributor

@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
Copy link

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

@AbhitejJohn
Copy link
Contributor

@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
Copy link

@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
Copy link
Contributor

@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
Copy link

NightOwl888 commented Aug 23, 2019

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
Copy link

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.

@NGloreous
Copy link
Contributor

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

@nohwnd nohwnd self-assigned this Jan 23, 2020
@maboivin
Copy link

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
Copy link
Member

nohwnd commented Feb 28, 2020

@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
Copy link
Contributor

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

@maboivin
Copy link

maboivin commented Apr 7, 2020

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

@poserdonut
Copy link

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
Copy link
Contributor

@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
Copy link

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

@enisak
Copy link

enisak commented Feb 9, 2022

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

Same here :(

@enisak
Copy link

enisak commented Jun 28, 2022

@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
Copy link

DimoYa commented Jan 18, 2023

any update ?

@Evangelink
Copy link
Member

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

@Evangelink Evangelink assigned Evangelink and unassigned nohwnd Jan 18, 2023
@Evangelink Evangelink added Needs: Author Feedback and removed Help-Wanted The issue is up-for-grabs, and can be claimed by commenting Type: Feature labels Jan 18, 2023
@NGloreous
Copy link
Contributor

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
Copy link
Member

Agreed, let me do that now.

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

No branches or pull requests