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

Implement Class Cleanup Lifecycle selection #968

Merged
merged 14 commits into from
Oct 27, 2021

Conversation

Haplois
Copy link
Contributor

@Haplois Haplois commented Sep 6, 2021

Originally posted by jhenkens on #724. Creating a new branch to be able to update it and merge.

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.

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal
@Haplois Haplois force-pushed the dev/haplois/class-cleanup-lifecycle branch from 0e10515 to ba15980 Compare September 6, 2021 10:03

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal
@Haplois Haplois force-pushed the dev/haplois/class-cleanup-lifecycle branch from 1a8e108 to cf5685e Compare September 6, 2021 13:21
@Haplois Haplois requested a review from nohwnd September 6, 2021 13:22
@Haplois Haplois force-pushed the dev/haplois/class-cleanup-lifecycle branch from cf5685e to bdd4a41 Compare September 6, 2021 13:35

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal
@Haplois Haplois enabled auto-merge (squash) September 9, 2021 12:21
@nohwnd
Copy link
Member

nohwnd commented Sep 9, 2021

I am missing some tests for the new behavior. There seem to be a lot of tests moved from one place to another. But are there any new tests to help us keep this in check?

@nohwnd
Copy link
Member

nohwnd commented Sep 10, 2021

Imho we should also take into account the Standard output here, which we would normally not send because we don't have the test object anymore, and send it because we do have the test object.

@Haplois Haplois added the sprint label Oct 18, 2021

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@Haplois
Copy link
Contributor Author

Haplois commented Oct 26, 2021

I am missing some tests for the new behavior. There seem to be a lot of tests moved from one place to another. But are there any new tests to help us keep this in check?

There isn't a way to test this behavior currently - I'll add some tests once we update our E2E test arch.

Imho we should also take into account the Standard output here, which we would normally not send because we don't have the test object anymore, and send it because we do have the test object.

We capture outputs if EndOfClass is selected, this is remarked in doc.

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal

Verified

This commit was signed with the committer’s verified signature.
Haplois Medeni Baykal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants