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

Add artifacts post processing #24012

Merged
merged 20 commits into from
Feb 20, 2022
Merged

Conversation

MarcoRossignoli
Copy link
Member

@MarcoRossignoli MarcoRossignoli commented Feb 18, 2022

contributes to microsoft/vstest#1811

Comment on lines +39 to +42
TestAsset testInstance = _testAssetsManager.CopyTestAsset("VSTestMultiProjectSolution", Guid.NewGuid().ToString())
.WithSource();

string runsettings = GetRunsetting(testInstance.Path);
Copy link
Member

Choose a reason for hiding this comment

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

THis is similar in all your tests, might be worth having a method for it? Maybe only the TestAsset creation.

Copy link
Member Author

@MarcoRossignoli MarcoRossignoli Feb 18, 2022

Choose a reason for hiding this comment

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

mmm what you mean?I need two item here testinstance and the runsettings file path for the command line, how would you organize?

Comment on lines +169 to +197
private void BuildDataCollector()
=> LazyInitializer.EnsureInitialized(ref s_dataCollectorDll, ref s_dataCollectorInitLock, () =>
{
TestAsset testInstance = _testAssetsManager.CopyTestAsset("VSTestDataCollectorSample").WithSource();

string testProjectDirectory = testInstance.Path;

new BuildCommand(testInstance)
.Execute("/p:Configuration=Release")
.Should()
.Pass();

return Directory.GetFiles(testProjectDirectory, "AttachmentProcessorDataCollector.dll", SearchOption.AllDirectories).Single(x => x.Contains("bin"));
});

private void BuildDataCollectorNoMerge()
=> LazyInitializer.EnsureInitialized(ref s_dataCollectorNoMergeDll, ref s_dataCollectorInitLock, () =>
{
TestAsset testInstance = _testAssetsManager.CopyTestAsset("VSTestDataCollectorSampleNoMerge").WithSource();

string testProjectDirectory = testInstance.Path;

new BuildCommand(testInstance)
.Execute("/p:Configuration=Release")
.Should()
.Pass();

return Directory.GetFiles(testProjectDirectory, "AttachmentProcessorDataCollector.dll", SearchOption.AllDirectories).Single(x => x.Contains("bin"));
});
Copy link
Member

Choose a reason for hiding this comment

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

This looks so weird. Why do we need static members that holds something that is changed for each test?

Copy link
Member Author

Choose a reason for hiding this comment

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

datacollectors are not changed for every tests, we reuse the same compilation for every tests(build on ctor). I prefer this instead of fixture to avoid the creation of class/apply interface etc...looks simpler to me.

MarcoRossignoli and others added 12 commits February 18, 2022 20:33
…ocessorDataCollector.csproj

Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
…hmentProcessorDataCollector.csproj

Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
…ctPostProcessing.cs

Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
…ctPostProcessing.cs

Co-authored-by: Amaury Levé <amaury.leve@gmail.com>
@MarcoRossignoli
Copy link
Member Author

@Evangelink refactored a bit the main Run() splitting between run for test containers and run with MSBuild, removed non safe write trace(I do the check inside there, we'll have more closure but it's ok).

@MarcoRossignoli MarcoRossignoli merged commit 2cc839f into dotnet:main Feb 20, 2022
@MarcoRossignoli MarcoRossignoli deleted the clipostproc branch February 20, 2022 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants