Extend Templates testing framework to test through API #5677

JanKrivanek · 2022-11-25T18:19:41Z

Problem

We are missing ability to leverage Templates Testing Tooling from within the Template Engine repository
This might be needed during moving samples: #2522

Solution

Extend TemplateVerifier to allow for custom instantiation callback
Add separate extension implementation performing instantiation through TemplateCreator API (not included within TemplateVerifier project to minimize dependecy pull for external scenarios)

Checks:

Added unit tests
Added #nullable enable to all the modified files ?

JanKrivanek · 2022-11-25T18:22:29Z

.../Microsoft.TemplateEngine.Authoring.TemplateVerifier.IntegrationTests/ExampleTemplateTest.cs

                UniqueFor = UniqueForOption.Architecture,
            }
+                .WithInstantiationThroughTemplateCreatorApi(new Dictionary<string, string?>() { { "paramB", "true" } })


@YuliiaKovalova this can be inspiration how the imported sample templates might be tested (only templateName, TemplatePath and WithInstantiationThroughTemplateCreatorApi will be needed - rest is here to unit test the actual testing functionality)

vlada-shubina

Please consider using IDE.Bootstrapper or full e2e via Edge for installing, loading and running the template.
The current logic likely deviates too much from regular experience, also doesn't support all the cases.

...IntegrationTests/Microsoft.TemplateEngine.Authoring.TemplateVerifier.IntegrationTests.csproj

src/Microsoft.TemplateEngine.Orchestrator.RunnableProjects/Global.cs

vlada-shubina · 2022-11-28T13:32:36Z

src/Microsoft.TemplateEngine.Edge/DefaultEnvironment.cs

@@ -18,7 +19,10 @@ public sealed class DefaultEnvironment : IEnvironment
        private const int DefaultBufferWidth = 160;
        private readonly IReadOnlyDictionary<string, string> _environmentVariables;

-        public DefaultEnvironment()
+        public DefaultEnvironment() : this(null)


just a thought - would it be better to do own implementation? Maybe something as ExtendedEnvironment?

another thought: https://github.com/dotnet/sdk/blob/main/src/Cli/Microsoft.TemplateEngine.Cli/CliEnvironment.cs we can get rid of this custom implementation with this class now.

There were few options:

Unseal the DefaultEnvironment and extend.

Copy paste DefaultEnvironment logic and add custom logic

Add the logic into DefaultEnvironment

The last one felt as the one with least dept incured. Though I might miss some obvious reason why it might not be an optional choice - so I'm open to alternative thoughts :-)

My preference is the first one, just to intentionally keep it separate, so it is not misused, however I don't insist.

test/Microsoft.TemplateEngine.TestHelper/EnvironmentSettingsHelper.cs

vlada-shubina · 2022-11-28T13:37:24Z

tools/Microsoft.TemplateEngine.Authoring.TemplateVerifier/Commands/IInstantiationResult.cs

+    /// <summary>
+    /// Exit code of the action (e.g. exit code of the dotnet new command).
+    /// 0 indicates successful action. Nonzero otherwise.
+    /// </summary>
+    int ExitCode { get; }
+
+    /// <summary>
+    /// Standard output stream content for the instantiation action.
+    /// </summary>
+    string StdOut { get; }
+
+    /// <summary>
+    /// Standard error stream content for the instantiation action.
+    /// </summary>
+    string StdErr { get; }


just a thought - does it make sense to be in an a separate interface?
IInstantiationResult
ICLIInstantiationResult : IInstantiationResult

.../Microsoft.TemplateEngine.Authoring.TemplateApiVerifier/TemplateVerifierOptionsExtensions.cs

vlada-shubina · 2022-11-28T13:52:46Z

Example using Bootstrapper is here:

templating/test/Microsoft.TemplateEngine.IDE.IntegrationTests/End2EndTests.cs

Lines 19 to 26 in 301c232

    
           using Bootstrapper bootstrapper = GetBootstrapper(); 
        
           string templateLocation = GetTestTemplateLocation("SourceNameForms"); 
        
           await InstallTemplateAsync(bootstrapper, templateLocation).ConfigureAwait(false); 
        
           string output = TestUtils.CreateTemporaryFolder(); 
        
           var foundTemplates = await bootstrapper.GetTemplatesAsync(new[] { WellKnownSearchFilters.NameFilter("TestAssets.SourceNameForms") }).ConfigureAwait(false); 
        
           var result = await bootstrapper.CreateAsync(foundTemplates[0].Info, "MyApp.1", output, new Dictionary<string, string?>()).ConfigureAwait(false);

vlada-shubina

Half way through, left some comments in, mostly typos.

src/Microsoft.TemplateEngine.IDE/Bootstrapper.cs

vlada-shubina · 2022-12-12T07:49:47Z

src/Microsoft.TemplateEngine.IDE/Bootstrapper.cs

+        /// <returns><see cref="ITemplateCreationResult"/> containing information on created template or error occurred.</returns>
+#pragma warning disable RS0027 // Public API with optional parameter(s) should have the most parameters amongst its public overloads
+#pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters
+        public Task<ITemplateCreationResult> CreateAsync(


just a thought - should we make other one obsolete?
also we are hardly using baselines, not sure if we should keep it and leave the warning.

The other CreateAsync (with params as dictionary) has legitimate use for quick testing with set of params - I'd keep it as is

src/Microsoft.TemplateEngine.Edge/DefaultEnvironment.cs

vlada-shubina · 2022-12-12T07:52:42Z

src/Microsoft.TemplateEngine.Edge/DefaultEnvironment.cs

@@ -18,7 +19,10 @@ public sealed class DefaultEnvironment : IEnvironment
        private const int DefaultBufferWidth = 160;
        private readonly IReadOnlyDictionary<string, string> _environmentVariables;

-        public DefaultEnvironment()
+        public DefaultEnvironment() : this(null)


My preference is the first one, just to intentionally keep it separate, so it is not misused, however I don't insist.

tools/Microsoft.TemplateEngine.Authoring.TemplateApiVerifier/LocalizableStrings.resx

vlada-shubina

Completed review - looks good, couple of minor comments and maybe some possible improvements for future.

.../Microsoft.TemplateEngine.Authoring.TemplateApiVerifier/TemplateVerifierOptionsExtensions.cs

vlada-shubina · 2022-12-12T17:22:28Z

.../Microsoft.TemplateEngine.Authoring.TemplateApiVerifier/TemplateVerifierOptionsExtensions.cs

+                result.Status == CreationResultStatus.Success ? string.Format(LocalizableStrings.CreateSuccessful, result.TemplateFullName) : string.Empty,
+                result.ErrorMessage ?? string.Empty,


Possible improvement: redirect logging here (info - output, error + warning - error).

JanKrivanek · 2022-12-13T15:08:53Z

/backport to release/7.0.2xx

github-actions · 2022-12-13T15:09:06Z

Started backporting to release/7.0.2xx: https://github.com/dotnet/templating/actions/runs/3686675295

github-actions · 2022-12-13T15:09:13Z

@JanKrivanek backporting to release/7.0.2xx failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Extend Templates testing framework to test through API
Applying: Reuse bootstrapper class
Applying: Bugfix after merge
error: sha1 information is lacking or useless (src/Microsoft.TemplateEngine.Edge/PublicAPI.Unshipped.txt).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0003 Bugfix after merge
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

github-actions · 2022-12-13T15:09:14Z

@JanKrivanek an error occurred while backporting to release/7.0.2xx, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

* Extend Templates testing framework to test through API * Reuse bootstrapper class * Bugfix after merge * Reflect PR comments * Add forgotten changes

JanKrivanek · 2022-12-14T17:48:14Z

/backport to release/7.0.2xx

github-actions · 2022-12-14T17:48:28Z

Started backporting to release/7.0.2xx: https://github.com/dotnet/templating/actions/runs/3697375728

github-actions · 2022-12-14T17:48:34Z

@JanKrivanek backporting to release/7.0.2xx failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Extend Templates testing framework to test through API
.git/rebase-apply/patch:296: trailing whitespace.
    
.git/rebase-apply/patch:297: trailing whitespace.
    
.git/rebase-apply/patch:309: trailing whitespace.
        
.git/rebase-apply/patch:311: trailing whitespace.
        
.git/rebase-apply/patch:313: trailing whitespace.
        
warning: squelched 36 whitespace errors
warning: 41 lines add whitespace errors.
Using index info to reconstruct a base tree...
M	tools/Microsoft.TemplateEngine.Authoring.TemplateVerifier/TemplateVerifierOptions.cs
M	tools/Microsoft.TemplateEngine.Authoring.TemplateVerifier/VerificationEngine.cs
Falling back to patching base and 3-way merge...
Auto-merging tools/Microsoft.TemplateEngine.Authoring.TemplateVerifier/VerificationEngine.cs
CONFLICT (content): Merge conflict in tools/Microsoft.TemplateEngine.Authoring.TemplateVerifier/VerificationEngine.cs
Auto-merging tools/Microsoft.TemplateEngine.Authoring.TemplateVerifier/TemplateVerifierOptions.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Extend Templates testing framework to test through API
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

github-actions · 2022-12-14T17:48:35Z

@JanKrivanek an error occurred while backporting to release/7.0.2xx, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

* Extend Templates testing framework to test through API * Reuse bootstrapper class * Bugfix after merge * Reflect PR comments * Add forgotten changes

…to release/7.0.2xx (#5766) * Extend Templates testing framework to test through API (#5677) * Extend Templates testing framework to test through API * Reuse bootstrapper class * Bugfix after merge * Reflect PR comments * Add forgotten changes * Fix contracts

Extend Templates testing framework to test through API

5a4f2e8

JanKrivanek requested a review from a team as a code owner November 25, 2022 18:19

JanKrivanek commented Nov 25, 2022

View reviewed changes

vlada-shubina requested changes Nov 28, 2022

View reviewed changes

JanKrivanek added 3 commits December 8, 2022 18:00

Reuse bootstrapper class

1eec8a5

Merge remote-tracking branch 'upstream/main' into te-api-testing

dc1ae71

Bugfix after merge

26b23e7

JanKrivanek requested a review from vlada-shubina December 8, 2022 17:10

vlada-shubina reviewed Dec 12, 2022

View reviewed changes

vlada-shubina approved these changes Dec 12, 2022

View reviewed changes

JanKrivanek added 2 commits December 13, 2022 11:51

Reflect PR comments

2d4fde5

Add forgotten changes

1a91b67

JanKrivanek merged commit 04c1ccf into dotnet:main Dec 13, 2022

JanKrivanek deleted the te-api-testing branch December 13, 2022 15:05

JanKrivanek mentioned this pull request Dec 13, 2022

Cherry pick [Extend Templates testing framework to test through API] to release/7.0.2xx #5756

Closed

JanKrivanek mentioned this pull request Dec 14, 2022

Cherry pick [Extend Templates testing framework to test through API] to release/7.0.2xx #5766

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend Templates testing framework to test through API #5677

Extend Templates testing framework to test through API #5677

JanKrivanek commented Nov 25, 2022

JanKrivanek Nov 25, 2022

vlada-shubina left a comment

vlada-shubina Nov 28, 2022

JanKrivanek Dec 8, 2022

vlada-shubina Dec 12, 2022

vlada-shubina Nov 28, 2022

vlada-shubina commented Nov 28, 2022

vlada-shubina left a comment

vlada-shubina Dec 12, 2022

JanKrivanek Dec 13, 2022

vlada-shubina Dec 12, 2022

vlada-shubina left a comment

vlada-shubina Dec 12, 2022

JanKrivanek commented Dec 13, 2022

github-actions bot commented Dec 13, 2022

github-actions bot commented Dec 13, 2022

github-actions bot commented Dec 13, 2022

JanKrivanek commented Dec 14, 2022

github-actions bot commented Dec 14, 2022

github-actions bot commented Dec 14, 2022

github-actions bot commented Dec 14, 2022

		result.Status == CreationResultStatus.Success ? string.Format(LocalizableStrings.CreateSuccessful, result.TemplateFullName) : string.Empty,
		result.ErrorMessage ?? string.Empty,

Extend Templates testing framework to test through API #5677

Extend Templates testing framework to test through API #5677

Conversation

JanKrivanek commented Nov 25, 2022

Problem

Solution

Checks:

Choose a reason for hiding this comment

vlada-shubina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vlada-shubina commented Nov 28, 2022

vlada-shubina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vlada-shubina left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JanKrivanek commented Dec 13, 2022

github-actions bot commented Dec 13, 2022

github-actions bot commented Dec 13, 2022

github-actions bot commented Dec 13, 2022

JanKrivanek commented Dec 14, 2022

github-actions bot commented Dec 14, 2022

github-actions bot commented Dec 14, 2022

github-actions bot commented Dec 14, 2022