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

Template verifier adoption in CommonTemplateTests #28707

Merged
merged 30 commits into from
Nov 25, 2022

Conversation

JanKrivanek
Copy link
Member

Problem

Part of dotnet/templating#3868
Coverting the CommonTemplateTests from manually running processes and checking outputs to use the verification tooling

Solution

Verification tooling importent and tests converted and adjusted.
Some repetitive tests (running identical scenarios but testing different parts of outputs) were consolidated

@JanKrivanek JanKrivanek requested a review from a team as a code owner October 21, 2022 13:02
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

@JanKrivanek JanKrivanek added Area-dotnet new the item is related to dotnet new command Area-Common templates Covers templates in the repo (classlib, console, common item templates) labels Oct 21, 2022
@JanKrivanek JanKrivanek changed the title Template verifier Template verifier adoption in CommonTemplateTests Oct 21, 2022
@JanKrivanek
Copy link
Member Author

@KlausLoeffelmann FYI

@JanKrivanek
Copy link
Member Author

JanKrivanek commented Oct 25, 2022

Restore action executed through the TemplateVerifier fails on Ubuntu and Darwin with

The "ProcessFrameworkReferences" task failed unexpectedly. [/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/3yvlfr5i.ltb/MyProject/console.csproj]
System.IO.FileLoadException: Could not load file or assembly 'NuGet.Frameworks, Version=6.5.0.35, Culture=neutral, PublicKeyToken=31bf3856ad364e35'. Could not find or load a specific file. (0x80131621) [/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/3yvlfr5i.ltb/MyProject/console.csproj]

NuGet.Frameworks is being transitively pulled by Microsoft.Dotnet.Cli.Utils - so waiting for this PR to be finished before proceeding with investigation: dotnet/templating#5482

Caused by not using the dotnet.exe from artifacts folder. To support custom dotnet location a VerificationEngine rework is needed that'll be done after dotnet/templating#5482

Copy link
Member

@vlada-shubina vlada-shubina left a comment

Choose a reason for hiding this comment

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

Left few comments.
In general, imo common tests should be just rewritten as snapshot tests testing all possible combination of parameters - it will cover all current tests.

src/Tests/dotnet-new.Tests/CommonTemplatesTests.cs Outdated Show resolved Hide resolved
src/Tests/dotnet-new.Tests/CommonTemplatesTests.cs Outdated Show resolved Hide resolved
src/Tests/dotnet-new.Tests/CommonTemplatesTests.cs Outdated Show resolved Hide resolved
src/Tests/dotnet-new.Tests/CommonTemplatesTests.cs Outdated Show resolved Hide resolved
src/Tests/dotnet-new.Tests/CommonTemplatesTests.cs Outdated Show resolved Hide resolved
@JanKrivanek
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@vlada-shubina vlada-shubina left a comment

Choose a reason for hiding this comment

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

I really like the simplification of the tests and decreased test run time!

Left couple of comments, all minor; some looks like possible issues.

@JanKrivanek
Copy link
Member Author

The last commit (3ccb7ba) addresses the #29197

[InlineData("global.json file", "globaljson", null)]
[InlineData("global.json file", "globaljson", new[] { "--sdk-version", "5.0.200" })]
[InlineData("global.json file", "globaljson", new[] { "--sdk-version", "5.0.200", "--roll-forward", "major" })]
[InlineData("global.json file", "global.json", null)]
[InlineData("global.json file", "global.json", new[] { "--sdk-version", "5.0.200" })]
Copy link
Member

Choose a reason for hiding this comment

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

we should remove .NET 5 here (EOL), can be done in another PR

@JanKrivanek
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JanKrivanek
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JanKrivanek JanKrivanek merged commit 8f2f52e into dotnet:main Nov 25, 2022
@JanKrivanek JanKrivanek deleted the template-verifier branch November 25, 2022 18:58
YuliiaKovalova pushed a commit to YuliiaKovalova/sdk that referenced this pull request Dec 15, 2022
Template verifier adoption in CommonTemplateTests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Common templates Covers templates in the repo (classlib, console, common item templates) Area-dotnet new the item is related to dotnet new command
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants