Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 311
Class Initialize inheritance #143#577
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
41362e38de73ffcd2b1e15d65c8000428759e2e6535e527d550aae98b258d16cf6f87261c1c36198b7acf95158832783e654ed1f95ffaeb663f38842f9264fa6dd345a25600682aad2c987ca72023d04138d28feFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -8,6 +8,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution | ||
| using System.Diagnostics; | ||
| using System.Diagnostics.CodeAnalysis; | ||
| using System.Globalization; | ||
| using System.Linq; | ||
| using System.Reflection; | ||
| using Extensions; | ||
| using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
| @@ -48,6 +49,8 @@ internal TestClassInfo( | ||
| this.ClassType = type; | ||
| this.Constructor = constructor; | ||
| this.TestContextProperty = testContextProperty; | ||
| this.BaseClassCleanupMethodsStack = new Stack<MethodInfo>(); | ||
| this.BaseClassInitAndCleanupMethods = new Queue<Tuple<MethodInfo, MethodInfo>>(); | ||
| this.BaseTestInitializeMethodsQueue = new Queue<MethodInfo>(); | ||
| this.BaseTestCleanupMethodsQueue = new Queue<MethodInfo>(); | ||
| this.Parent = parent; | ||
| @@ -107,6 +110,11 @@ internal set | ||
| /// </summary> | ||
| public bool IsClassInitializeExecuted { get; internal set; } | ||
| /// <summary> | ||
| /// Gets a stack of class cleanup methods to be executed. | ||
| /// </summary> | ||
| public Stack<MethodInfo> BaseClassCleanupMethodsStack { get; internal set; } | ||
| /// <summary> | ||
| /// Gets the exception thrown during <see cref="ClassInitializeAttribute"/> method invocation. | ||
| /// </summary> | ||
| @@ -157,6 +165,11 @@ public bool HasExecutableCleanupMethod | ||
| } | ||
| } | ||
| /// <summary> | ||
| /// Gets a tuples' queue of class initialize/cleanup methods to call for this type. | ||
| /// </summary> | ||
| public Queue<Tuple<MethodInfo, MethodInfo>> BaseClassInitAndCleanupMethods { get; private set; } | ||
| /// <summary> | ||
| /// Gets the test initialize method. | ||
| /// </summary> | ||
| @@ -219,8 +232,8 @@ internal set | ||
| [SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle all kinds of user exceptions and message appropriately.")] | ||
| public void RunClassInitialize(TestContext testContext) | ||
| { | ||
| // If no class initialize return | ||
| if (this.ClassInitializeMethod == null) | ||
| // If no class initialize and no base class initialize, return | ||
| if (this.ClassInitializeMethod is null && !this.BaseClassInitAndCleanupMethods.Any(p => p.Item1 != null)) | ||
| { | ||
| return; | ||
| } | ||
| @@ -230,8 +243,50 @@ public void RunClassInitialize(TestContext testContext) | ||
| throw new NullReferenceException(Resource.TestContextIsNull); | ||
| } | ||
| // If class initialization is not done, then do it. | ||
| if (!this.IsClassInitializeExecuted) | ||
| MethodInfo initializeMethod = null; | ||
| string failedClassInitializeMethodName = string.Empty; | ||
| // If class initialization is done, just return | ||
| if (this.IsClassInitializeExecuted) | ||
| { | ||
| return; | ||
| } | ||
| // Aquiring a lock is usually a costly operation which does not need to be | ||
| // performed every time if the class init is already executed. | ||
| lock (this.testClassExecuteSyncObject) | ||
| { | ||
| // Perform a check again. | ||
| if (this.IsClassInitializeExecuted) | ||
| { | ||
| return; | ||
| } | ||
| try | ||
| { | ||
| // ClassInitialize methods for base classes are called in reverse order of discovery | ||
| // Base -> Child TestClass | ||
| var baseClassInitializeStack = new Stack<Tuple<MethodInfo, MethodInfo>>( | ||
| this.BaseClassInitAndCleanupMethods.Where(p => p.Item1 != null)); | ||
| while (baseClassInitializeStack.Count > 0) | ||
| { | ||
| var baseInitCleanupMethods = baseClassInitializeStack.Pop(); | ||
| initializeMethod = baseInitCleanupMethods.Item1; | ||
| initializeMethod?.InvokeAsSynchronousTask(null, testContext); | ||
| this.BaseClassCleanupMethodsStack.Push(baseInitCleanupMethods.Item2); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| this.ClassInitializationException = ex; | ||
| failedClassInitializeMethodName = initializeMethod.Name; | ||
| } | ||
| } | ||
| // If class initialization is not done and class initialize method is not null, | ||
| // and class initialization exception is null, then do it. | ||
| if (!this.IsClassInitializeExecuted && this.classInitializeMethod != null && this.ClassInitializationException == null) | ||
| { | ||
| // Aquiring a lock is usually a costly operation which does not need to be | ||
parrainc marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| // performed every time if the class init is already executed. | ||
| @@ -247,6 +302,7 @@ public void RunClassInitialize(TestContext testContext) | ||
| catch (Exception ex) | ||
| { | ||
| this.ClassInitializationException = ex; | ||
| failedClassInitializeMethodName = this.ClassInitializeMethod.Name; | ||
| } | ||
| finally | ||
| { | ||
| @@ -271,15 +327,13 @@ public void RunClassInitialize(TestContext testContext) | ||
| var realException = this.ClassInitializationException.InnerException ?? this.ClassInitializationException; | ||
| var outcome = UnitTestOutcome.Failed; | ||
| string errorMessage = null; | ||
| StackTraceInformation exceptionStackTraceInfo = null; | ||
| if (!realException.TryGetUnitTestAssertException(out outcome, out errorMessage, out exceptionStackTraceInfo)) | ||
| if (!realException.TryGetUnitTestAssertException(out outcome, out string errorMessage, out StackTraceInformation exceptionStackTraceInfo)) | ||
| { | ||
| errorMessage = string.Format( | ||
| CultureInfo.CurrentCulture, | ||
| Resource.UTA_ClassInitMethodThrows, | ||
| this.ClassType.FullName, | ||
| this.ClassInitializeMethod.Name, | ||
| failedClassInitializeMethodName, | ||
| realException.GetType().ToString(), | ||
| StackTraceHelper.GetExceptionMessage(realException)); | ||
| @@ -298,19 +352,26 @@ public void RunClassInitialize(TestContext testContext) | ||
| /// <returns> | ||
| /// Any exception that can be thrown as part of a class cleanup as warning messages. | ||
| /// </returns> | ||
| [SuppressMessageAttribute("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle all kinds of user exceptions and message appropriately.")] | ||
| [SuppressMessage("Microsoft.Design", "CA1031:DoNotCatchGeneralExceptionTypes", Justification = "Requirement is to handle all kinds of user exceptions and message appropriately.")] | ||
parrainc marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| public string RunClassCleanup() | ||
| { | ||
| if (this.ClassCleanupMethod == null) | ||
| if (this.ClassCleanupMethod is null && !this.BaseClassInitAndCleanupMethods.Any(p => p.Item2 != null)) | ||
parrainc marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| { | ||
| return null; | ||
| } | ||
| if (this.IsClassInitializeExecuted || this.ClassInitializeMethod == null) | ||
| if (this.IsClassInitializeExecuted || this.ClassInitializeMethod is null || this.BaseClassCleanupMethodsStack.Any()) | ||
| { | ||
| var classCleanupMethod = this.ClassCleanupMethod; | ||
| try | ||
| { | ||
| this.ClassCleanupMethod.InvokeAsSynchronousTask(null); | ||
| classCleanupMethod?.InvokeAsSynchronousTask(null); | ||
| var baseClassCleanupQueue = new Queue<MethodInfo>(this.BaseClassCleanupMethodsStack); | ||
| while (baseClassCleanupQueue.Count > 0) | ||
| { | ||
| classCleanupMethod = baseClassCleanupQueue.Dequeue(); | ||
| classCleanupMethod?.InvokeAsSynchronousTask(null); | ||
Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't we be only executing cleanups whose inits have run? This seems to be running all cleanups. Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we could just push Cleanups that need to be executed into a Stack instead of ContributorAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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: Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. It doesn't look like we need a Queue here. We could just pop the methods off the stack.. | ||
| } | ||
| return null; | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.