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

WIP: Implement Class Cleanup Lifecycle selection #724

Closed
wants to merge 1 commit into from

Conversation

jhenkens
Copy link

I'm soliciting early feedback on a branch to introduce a lifecycle policy for the ClassCleanup attribute. This allows a test author to set at the command line, assembly, or class level, whether a the ClassCleanup should happen at the end of a class, or at the end of the assembly.

I've lightly tested this, and it seems to work as expected in my tests, though I am not confident in its robustness in its current form. Specifically, my current implementation relying on a dictionary of the strings of the full test names seems fragile to me, though I was unable to determine how to instead track via the guids on the completion side of the dictionary.

If this feature seems generally appeciated, I'd love guidance from those more familar with the MSTest codebase in how I might improve my implementation.

A part of any final submission I will surely include reverting the many changed build files - those were needed to build in my environment, and are not to be present in any final reviews.

Thank you.

@ghost
Copy link

ghost commented Jul 16, 2020

CLA assistant check
All CLA requirements met.

Copy link
Member

@nohwnd nohwnd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some early thoughts, I did not get to dig deeper into the code and think about edge cases yet, unfortunately.

@@ -56,6 +56,13 @@ internal TestAssemblySettings GetSettings(string source)

testAssemblySettings.CanParallelizeAssembly = !this.reflectHelper.IsDoNotParallelizeSet(testAssembly);

var classCleanupSequencingAttribute = this.reflectHelper.GetClassCleanupAttribute(testAssembly);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this naming based on some existing settings we have? I think we should choose between Sequencing and Lifecycle if there is no existing example of this and in that case Lifecycle seems easier to grasp.

Or possibly ClassCleanupExecution?

@Haplois
Copy link
Contributor

Haplois commented Sep 6, 2021

Code moved to #968. @jhenkens thank you for the contribution!

@Haplois Haplois closed this Sep 6, 2021
Haplois added a commit that referenced this pull request Sep 6, 2021
Haplois added a commit that referenced this pull request Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants