Class Initialize inheritance #143 #577

parrainc · 2019-03-10T20:06:19Z

Implemented feature requested in #143

Create enum to handle the inheritance option in ClassInitialize attribute
Set None option to default in ClassInitialize attribute
Added properties on TestClassInfo class
Updated RunClassInitialize and RunClassCleanup method in TestClassInfo class
Updated CreateClassInfo method on TypeCache class to check for classinitialize inheritance option
Added a few unit tests for TestClassInfo and TypeCache

example output:
===> Init from ParentTestClass
==> Init from BaseTestClass
=> Init from DerivedTestClass

=> Cleaning up from DerivedTestClass
==> Cleaning up from BaseTestClass
===> Cleaning up from ParentTestClass

…ue143-classinitializeinh

…arnings

parrainc · 2019-03-10T20:07:48Z

cc: @AbhitejJohn @jayaranigarg @singhsarab
Please, review...

…g run base init

parrainc · 2019-03-10T23:19:19Z

Pushed new changes to fix that base cleanup were being called without calling base initialize

AbhitejJohn

Thanks for putting this out. Added my feedback. @acesiddhu or @jayaranigarg would be able to address the framework attribute concern better but looks good overall.

src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs

src/Adapter/MSTest.CoreAdapter/Execution/TypeCache.cs

src/TestFramework/MSTest.Core/Attributes/VSTestAttributes.cs

src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs

src/TestFramework/MSTest.Core/Attributes/VSTestAttributes.cs

…ue143-classinitializeinh

parrainc · 2019-03-14T22:43:45Z

@dotnet-bot test Windows / Release Build please

…ue143-classinitializeinh

…ainc/testfx into issue143-classinitializeinh

sturlath · 2019-04-09T10:20:08Z

So when can we expect this to be merged? I really need to be able to use [ClassInitialize] in my custom base class and if I'm correct this will make that possible. Right?

…ue143-classinitializeinh

…ainc/testfx into issue143-classinitializeinh

parrainc · 2019-04-10T00:24:49Z

@sturlath

So when can we expect this to be merged? I really need to be able to use [ClassInitialize] in my custom base class and if I'm correct this will make that possible. Right?

From my side, I can tell you that since I've been kind of busy, i'm working a little bit slower than before on this, but even though, I think by end of next week I'll be done with this (unless there's more review to solve).

This changes will let you tell the framework to take into account your custom base class' initialize method when running your tests. So, yeah, you're correct.

I think @jayaranigarg and @AbhitejJohn could expand a bit more when this would be released after merged into master.

AbhitejJohn

Thanks for updating the logic. It might need a few minor touch-ups and testing but apart from the comments I've put in, it looks good. I'm hoping @jayaranigarg or @cltshivash would be able to help with approvals and merging. I'm just helping as a fellow open source-rer and cannot merge this in myself. I can help with the cleanups if I need to, to get this in quicker.

src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs

AbhitejJohn · 2019-04-11T05:26:55Z

src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs

+                        initializeMethod = baseClassInitializeStack.Pop().Item1;
+                        if (initializeMethod != null)
+                        {
+                            initializeMethod?.InvokeAsSynchronousTask(null, testContext);


We do not need this null check operator here given that we are already performing a null check above.

actually, i think we might needed it since the null check above is to make sure there are any non-null init method in the tuple that can be executed. In any case, I changed this for a .Where call when newing up the baseClassInitializeStack (on the this.BaseClassInitAndCleanupMethods) that way, we just pass executable init methods, and we don't need to do any extra thing to pass its corresponding cleanup method to the stack for cleanup methods to execute in the RunClassCleanup method.

It looks like the updated code wouldn't run Cleanups from base classes that do not have an initialize method...

src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs

AbhitejJohn · 2019-04-11T05:45:28Z

src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs

+                    while (baseClassCleanupQueue.Count > 0)
+                    {
+                        classCleanupMethod = baseClassCleanupQueue.Dequeue().Item2;
+                        classCleanupMethod?.InvokeAsSynchronousTask(null);


shouldn't we be only executing cleanups whose inits have run? This seems to be running all cleanups.

Maybe we could just push Cleanups that need to be executed into a Stack instead of ExecutedBaseClassInitializeMethods

Yup, I had a block of code that I removed right before pushing the latest changes, handling the logic for deciding those cleanup method that will be executed. But, now that you mentioned that last part, I think that'd be better. For that, renamed the property you mentioned, now it's called: BaseClassCleanupMethodsStack not sure if the name is 100% ok, and in the RunClassInitialize method, I'll be pushing the cleanups into that stack to then execute them in the RunClassCleanup method.

Sounds good. It doesn't look like we need a Queue here. We could just pop the methods off the stack..

src/TestFramework/MSTest.Core/Attributes/VSTestAttributes.cs

paulmccormack · 2019-05-23T06:52:43Z

Hi

Do you know when this change will be merged and released?

Cheers

Paul

AbhitejJohn · 2019-05-23T08:04:29Z

I'd like to see this in too.
/cc: @acesiddhu , @singhsarab .

NGloreous · 2019-06-20T19:52:24Z

@parrainc Any updates on this change? Look like there hasn't been progress for a bit. I'm in need of this and would be happy to help if needed.

AbhitejJohn · 2019-06-27T18:10:54Z

So, I've checked offline with the team on getting this in. The plan is to do the needful to merge this in early next week. I believe @parrainc has addressed most of the feedback here and I'd love to have this in the product.
/cc: @cltshivash , @jayaranigarg

parrainc · 2019-07-02T16:38:49Z

I think most of the comments (if not, all of them) were addressed. I've been a little busy but if something still missing I could fix it, so it could be merged. So, basically, I'm waiting on @jayaranigarg @cltshivash inputs. If this looks good for them as well, then we should be good.

jayaranigarg · 2019-07-08T10:27:47Z

@sturlath @paulmccormack @NGloreous You can 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

elgatov · 2019-09-19T19:40:24Z

@parrainc must classinitialize be static for it to run? i do not have it marked static and it will not run from a base class

parrainc · 2019-09-30T00:18:01Z

@parrainc must classinitialize be static for it to run? i do not have it marked static and it will not run from a base class

@elgatov yes, it must has this signature:

[ClassInitialize(InheritanceBehavior.BeforeEachDerivedClass)]
public static void InitBaseClassMethod(TestContext testContext) { }

parrainc added 6 commits November 7, 2018 09:04

Implemented Initialize Inheritance for ClassInitialize attribute

41362e3

Implemented Cleanup inheritance behavior for ClassCleanup attr

8de73ff

Merge branch 'master' of https://github.com/Microsoft/testfx into iss…

cd2b1e1

…ue143-classinitializeinh

Merge branch 'master' of https://github.com/Microsoft/testfx into iss…

5d65c80

…ue143-classinitializeinh

Removed OnceBeforeAnyDerivedClasses from ClassInitializeInheritance enum

0042875

Changed UTF.TestClass for custom testclass attr to avoid test ouput w…

9e2e653

…arnings

parrainc added 2 commits March 10, 2019 19:00

Updated RunClassCleanup to prevent running base cleanup without havin…

5e527d5

…g run base init

Fix name typo

50aae98

added doc for BaseClassInitializeMethodsDict prop

b258d16

singhsarab requested review from jayaranigarg and acesiddhu March 11, 2019 06:39

AbhitejJohn reviewed Mar 14, 2019

View reviewed changes

jayaranigarg reviewed Mar 14, 2019

View reviewed changes

src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs Outdated Show resolved Hide resolved

src/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs Outdated Show resolved Hide resolved

src/TestFramework/MSTest.Core/Attributes/VSTestAttributes.cs Outdated Show resolved Hide resolved

parrainc added 2 commits March 14, 2019 18:10

Updates per review comments

cf6f872

Merge branch 'master' of https://github.com/Microsoft/testfx into iss…

61c1c36

…ue143-classinitializeinh

jayaranigarg and others added 3 commits April 3, 2019 10:01

Merge branch 'master' into issue143-classinitializeinh

198b7ac

Merge branch 'master' of https://github.com/Microsoft/testfx into iss…

f951588

…ue143-classinitializeinh

Merge branch 'issue143-classinitializeinh' of https://github.com/parr…

32783e6

…ainc/testfx into issue143-classinitializeinh

jayaranigarg and others added 4 commits April 9, 2019 18:17

Merge branch 'master' into issue143-classinitializeinh

54ed1f9

Merge branch 'master' into issue143-classinitializeinh

5ffaeb6

Merge branch 'master' of https://github.com/Microsoft/testfx into iss…

63f3884

…ue143-classinitializeinh

Merge branch 'issue143-classinitializeinh' of https://github.com/parr…

2f9264f

…ainc/testfx into issue143-classinitializeinh

parrainc added 2 commits April 9, 2019 21:06

making inheritance behavior enum more generic and updating tests

a6dd345

done some refactoring and added/fixed a few tests

a256006

AbhitejJohn approved these changes Apr 11, 2019

View reviewed changes

parrainc added 4 commits April 13, 2019 15:43

updates as per review comments

82aad2c

updated classinfo and tests

987ca72

updated classcleanup 1-param ctor

023d041

updating doc message for 1-param ctors

38d28fe

jayaranigarg merged commit c590c2b into microsoft:master Jul 8, 2019

abhijitpai mentioned this pull request May 26, 2020

When ClassInitialize fails only one test fails, others executed regardless #672

Closed

nohwnd mentioned this pull request Dec 14, 2023

ClassCleanup not called in base class #1889

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Class Initialize inheritance #143 #577

Class Initialize inheritance #143 #577

parrainc commented Mar 10, 2019 •

edited

Loading

parrainc commented Mar 10, 2019

parrainc commented Mar 10, 2019

AbhitejJohn left a comment

parrainc commented Mar 14, 2019

sturlath commented Apr 9, 2019

parrainc commented Apr 10, 2019

AbhitejJohn left a comment

AbhitejJohn Apr 11, 2019

parrainc Apr 13, 2019

AbhitejJohn Apr 15, 2019

AbhitejJohn Apr 11, 2019

AbhitejJohn Apr 11, 2019

parrainc Apr 13, 2019

AbhitejJohn Apr 15, 2019

paulmccormack commented May 23, 2019

AbhitejJohn commented May 23, 2019

NGloreous commented Jun 20, 2019

AbhitejJohn commented Jun 27, 2019

parrainc commented Jul 2, 2019

jayaranigarg commented Jul 8, 2019

elgatov commented Sep 19, 2019

parrainc commented Sep 30, 2019 •

edited

Loading

Class Initialize inheritance #143 #577

Class Initialize inheritance #143 #577

Conversation

parrainc commented Mar 10, 2019 • edited Loading

parrainc commented Mar 10, 2019

parrainc commented Mar 10, 2019

AbhitejJohn left a comment

Choose a reason for hiding this comment

parrainc commented Mar 14, 2019

sturlath commented Apr 9, 2019

parrainc commented Apr 10, 2019

AbhitejJohn left a comment

Choose a reason for hiding this comment

AbhitejJohn Apr 11, 2019

Choose a reason for hiding this comment

parrainc Apr 13, 2019

Choose a reason for hiding this comment

AbhitejJohn Apr 15, 2019

Choose a reason for hiding this comment

AbhitejJohn Apr 11, 2019

Choose a reason for hiding this comment

AbhitejJohn Apr 11, 2019

Choose a reason for hiding this comment

parrainc Apr 13, 2019

Choose a reason for hiding this comment

AbhitejJohn Apr 15, 2019

Choose a reason for hiding this comment

paulmccormack commented May 23, 2019

AbhitejJohn commented May 23, 2019

NGloreous commented Jun 20, 2019

AbhitejJohn commented Jun 27, 2019

parrainc commented Jul 2, 2019

jayaranigarg commented Jul 8, 2019

elgatov commented Sep 19, 2019

parrainc commented Sep 30, 2019 • edited Loading

parrainc commented Mar 10, 2019 •

edited

Loading

parrainc commented Sep 30, 2019 •

edited

Loading