Class Initialize inheritance #143 by parrainc · Pull Request #577 · microsoft/testfx

Carlos Parra (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

Carlos Parra (parrainc) · 2019-03-10T20:07:48Z

cc: Abhitej John (@AbhitejJohn)Jayarani garg (@jayaranigarg)Sarabjot Singh (@singhsarab)
Please, review...

…g run base init

Carlos Parra (parrainc) · 2019-03-10T23:19:19Z

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

Abhitej John (AbhitejJohn)

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

…ue143-classinitializeinh

Carlos Parra (parrainc) · 2019-03-14T22:43:45Z

dotnet bot (@dotnet-bot) test Windows / Release Build please

…ue143-classinitializeinh

…ainc/testfx into issue143-classinitializeinh

Sturla Thorvaldsson (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

Carlos Parra (parrainc) · 2019-04-10T00:24:49Z

Sturla Thorvaldsson (@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 Jayarani garg (@jayaranigarg) and Abhitej John (@AbhitejJohn) could expand a bit more when this would be released after merged into master.

Abhitej John (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 Jayarani garg (@jayaranigarg) or Shiva Shankar (@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.

Abhitej John (AbhitejJohn) · 2019-04-11T05:26:55Z

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

Abhitej John (AbhitejJohn) · 2019-04-11T05:45:28Z

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

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

Hi

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

Cheers

Paul

Abhitej John (AbhitejJohn) · 2019-05-23T08:04:29Z

I'd like to see this in too.
/cc: Siddhartha Pandey (@acesiddhu) , Sarabjot Singh (@singhsarab) .

Nolan Glore (NGloreous) · 2019-06-20T19:52:24Z

Carlos Parra (@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.

Abhitej John (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 Carlos Parra (@parrainc) has addressed most of the feedback here and I'd love to have this in the product.
/cc: Shiva Shankar (@cltshivash) , Jayarani garg (@jayaranigarg)

Carlos Parra (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 Jayarani garg (@jayaranigarg)Shiva Shankar (@cltshivash) inputs. If this looks good for them as well, then we should be good.

Jayarani garg (jayaranigarg) · 2019-07-08T10:27:47Z

Sturla Thorvaldsson (@sturlath)paulmccormack Nolan Glore (@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 (elgatov) · 2019-09-19T19:40:24Z

Carlos Parra (@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

Carlos Parra (parrainc) · 2019-09-30T00:18:01Z

Carlos Parra (@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 (@elgatov) yes, it must has this signature:

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

Carlos Parra (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' ofhttps://github.com/Microsoft/testfxinto iss…
cd2b1e1
…ue143-classinitializeinh

Merge branch 'master' ofhttps://github.com/Microsoft/testfxinto iss…
5d65c80
…ue143-classinitializeinh

Removed OnceBeforeAnyDerivedClasses from ClassInitializeInheritance enum
0042875

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

Carlos Parra (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

Sarabjot Singh (singhsarab) requested review from Siddhartha Pandey (acesiddhu) and Jayarani garg (jayaranigarg)March 11, 2019 06:39

Abhitej John (AbhitejJohn) reviewed Mar 14, 2019
View reviewed changes

Jayarani garg (jayaranigarg) reviewed Mar 14, 2019
View reviewed changes

Comment threadsrc/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs Outdated
Comment threadsrc/Adapter/MSTest.CoreAdapter/Execution/TestClassInfo.cs Outdated
Comment threadsrc/TestFramework/MSTest.Core/Attributes/VSTestAttributes.cs Outdated

Carlos Parra (parrainc) added 2 commits March 14, 2019 18:10

Updates per review comments
cf6f872

Merge branch 'master' ofhttps://github.com/Microsoft/testfxinto iss…
61c1c36
…ue143-classinitializeinh

Jayarani garg (jayaranigarg)and others added 3 commits April 3, 2019 10:01

Merge branch 'master' into issue143-classinitializeinh
198b7ac

Merge branch 'master' ofhttps://github.com/Microsoft/testfxinto iss…
f951588
…ue143-classinitializeinh

Merge branch 'issue143-classinitializeinh' ofhttps://github.com/parr…
32783e6
…ainc/testfx into issue143-classinitializeinh

Jayarani garg (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' ofhttps://github.com/Microsoft/testfxinto iss…
63f3884
…ue143-classinitializeinh

Merge branch 'issue143-classinitializeinh' ofhttps://github.com/parr…
2f9264f
…ainc/testfx into issue143-classinitializeinh

Carlos Parra (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

Abhitej John (AbhitejJohn) approved these changes Apr 11, 2019
View reviewed changes

Carlos Parra (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

Jayarani garg (jayaranigarg) merged commit c590c2b into microsoft:masterJul 8, 2019

abhijitpai mentioned this pull request May 26, 2020
When ClassInitialize fails only one test fails, others executed regardless #672
Closed

Jakub Jareš (nohwnd) mentioned this pull request Dec 14, 2023
ClassCleanup not called in base class #1889
Closed

Uh oh!

Conversation

Carlos Parra (parrainc) commented Mar 10, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Carlos Parra (parrainc) commented Mar 10, 2019

Uh oh!

Carlos Parra (parrainc) commented Mar 10, 2019

Uh oh!

Abhitej John (AbhitejJohn) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Carlos Parra (parrainc) commented Mar 14, 2019

Uh oh!

Sturla Thorvaldsson (sturlath) commented Apr 9, 2019

Uh oh!

Carlos Parra (parrainc) commented Apr 10, 2019

Uh oh!

Abhitej John (AbhitejJohn) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Abhitej John (AbhitejJohn)Apr 11, 2019

Choose a reason for hiding this comment

Uh oh!

Carlos Parra (parrainc)Apr 13, 2019

Choose a reason for hiding this comment

Uh oh!

Abhitej John (AbhitejJohn)Apr 15, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Abhitej John (AbhitejJohn)Apr 11, 2019

Choose a reason for hiding this comment

Uh oh!

Abhitej John (AbhitejJohn)Apr 11, 2019

Choose a reason for hiding this comment

Uh oh!

Carlos Parra (parrainc)Apr 13, 2019

Choose a reason for hiding this comment

Uh oh!

Abhitej John (AbhitejJohn)Apr 15, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paulmccormack commented May 23, 2019

Uh oh!

Abhitej John (AbhitejJohn) commented May 23, 2019

Uh oh!

Nolan Glore (NGloreous) commented Jun 20, 2019

Uh oh!

Abhitej John (AbhitejJohn) commented Jun 27, 2019

Uh oh!

Carlos Parra (parrainc) commented Jul 2, 2019

Uh oh!

Jayarani garg (jayaranigarg) commented Jul 8, 2019

Uh oh!

elgatov (elgatov) commented Sep 19, 2019

Uh oh!

Carlos Parra (parrainc) commented Sep 30, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Carlos Parra (parrainc) commented Mar 10, 2019•
edited
Loading

Carlos Parra (parrainc) commented Sep 30, 2019•
edited
Loading