Test all links in templates #8628

ryanbrandenburg · 2019-03-18T16:33:29Z

Also added some tests for some scenarios that were missing.

src/ProjectTemplates/test/EmptyWebTemplateTest.cs

ryanbrandenburg · 2019-03-25T19:17:51Z

src/ProjectTemplates/test/Helpers/AspNetProcess.cs

javiercn · 2019-03-25T19:31:46Z

I don't think that this PR addresses aspnet/Templating#692 correctly. The reason is because when a link doesn't exist on routing we will generate an empty link, which will return a 200 status code when visited as its pointing to the same page you are on.

I think the only correct way to do this is to collect all the links on a given page and assert they are present. And then after that, if you want to, you visit them.

For example:

aspNetProcess.ContainsLinks("/", expectedLinks)
aspNetProcess.AssertOk("/Privacy")

To be clear, this test will pass in many broken scenarios, so we should be more specific on what we are testing. Specially given the fact that our templates only contain a small set of urls.

ryanbrandenburg · 2019-03-25T19:40:06Z

Fair duce. We do also want to confirm that external links don't 404 (it's happened before) but you're right that given that Routing behavior this PR isn't covering the most likely break-case.

pranavkm · 2019-03-26T22:51:28Z

<AddRazorSupportForMvc Condition="'$(SupportPagesAndViews)' == 'True'">>true</AddRazorSupportForMvc>

pranavkm · 2019-03-26T22:55:54Z

src/ProjectTemplates/Web.ProjectTemplates/RazorClassLibrary-CSharp.csproj.in

@@ -5,6 +5,7 @@
    <TargetFramework Condition="'$(SupportPagesAndViews)' != 'True'">netstandard2.0</TargetFramework>
    <RazorLangVersion Condition="'$(SupportPagesAndViews)' != 'True'">3.0</RazorLangVersion>
    <RootNamespace Condition="'$(name)' != '$(name{-VALUE-FORMS-}safe_namespace)'">Company.RazorClassLibrary1</RootNamespace>
+    <AddRazorSupportForMvc Condition="'$(SupportPagesAndViews)' == 'True'">true</AddRazorSupportForMvc>


javiercn · 2019-03-28T09:52:29Z

src/ProjectTemplates/test/GrpcTemplateTest.cs

+
+        [Fact]
+        public async Task GrpcTemplate()
+        {


Is this a class-library or an executable, if it's the former. Should we test it runs?

/cc: @glennc

I didn't add that because I don't know what the expected behavior is. Would certainly be reasonable to at least check it doesn't crash instantly and file an issue for someone involved with Grpc to test expectations.

javiercn · 2019-03-28T10:03:08Z

src/ProjectTemplates/test/Helpers/AspNetProcess.cs

+            }
+        }
+
+        public async Task ContainsLinks(string requestUrl, IEnumerable<string> expectedLinks)


I'm not sure this is covering things like <link rel="stylesheet" type="text/css" href="theme.css"> and similar. Or <script src="app.js" /> or for that matter. Are we not planning to cover those?

Also, shouldn't we fail if we encounter more links than expected? That would mean that we added something we are not testing.

This is definitely not covering thinks like missing style-sheets/scripts. My thought was to cover that using the Browser console checks we have to add anyway for the LibMan switch, but that would mean testing non-SPA templates with Selenium. May as well start doing it here since we're already parsing.

javiercn · 2019-03-28T10:04:08Z

src/ProjectTemplates/test/WorkerTemplateTest.cs

+
+        [Fact(Skip = "Microsoft.NET.Sdk.Worker isn't available yet")]
+        public async Task WorkerTemplateAsync()
+        {


Same comment as for the grpc template. Shouldn't we run this if it's an executable template?

/cc: @glennc

javiercn

I only have a comment about the links regarded to link and script elements and the fact that we don't fail if there are additional links than the ones we expect. Other than that and a potentially missing couple of test actions looks good. once we have a resolution for those three items.

urls in link and script tags.
fail when we find additional links.
running the grpc and worker templates.

ryanbrandenburg · 2019-04-01T23:55:58Z

🆙📅

javiercn · 2019-04-02T18:16:12Z

src/ProjectTemplates/test/GrpcTemplateTest.cs

+                using (var clientProcess = Project.StartBuiltClientAsync(serverProcess))
+                {
+                    // Wait for the client to do its thing
+                    System.Threading.Thread.Sleep(100);


await Task.Delay(100)

javiercn · 2019-04-02T18:17:47Z

src/ProjectTemplates/test/Helpers/AspNetProcess.cs

        internal ProcessEx Process { get; }

        public AspNetProcess(
            ITestOutputHelper output,
            string workingDirectory,
            string dllPath,
-            IDictionary<string, string> environmentVariables)
+            IDictionary<string, string> environmentVariables,
+            bool useExec = true,


can you rename useExec to published? that's essentially what it is.

javiercn · 2019-04-02T18:20:19Z

src/ProjectTemplates/test/Helpers/Project.cs

+            return new AspNetProcess(Output, TemplateClientReleaseDir, projectDll, environment);
+        }
+
+        private const string _urls = "http://127.0.0.1:0;https://127.0.0.1:0";


this should go at the top of the file.

javiercn · 2019-04-02T18:20:58Z

src/ProjectTemplates/test/RazorPagesTemplateTest.cs

@@ -23,6 +24,7 @@ public RazorPagesTemplateTest(ProjectFactoryFixture projectFactory, ITestOutputH

        public ITestOutputHelper Output { get; }

+


nit: unnecessary extra line

javiercn · 2019-04-02T18:21:19Z

src/ProjectTemplates/test/RazorPagesTemplateTest.cs

@@ -31,8 +33,6 @@ public async Task RazorPagesTemplate_NoAuthImplAsync()
            var createResult = await Project.RunDotNetNewAsync("razor");
            Assert.True(0 == createResult.ExitCode, ErrorMessages.GetFailedProcessMessage("razor", Project, createResult));

-            AssertFileExists(Project.TemplateOutputDir, "Pages/Shared/_LoginPartial.cshtml", false);


Why did we remove this check?

File existence is covered more thoroughly here so I figured it was best to only test it in one place.

javiercn

Some cleanups left, but otherwise looks good!

ryanbrandenburg · 2019-04-02T22:20:13Z

@aspnet-hello triage https://dev.azure.com/dnceng/public/_build/results?buildId=140765

aspnet-hello · 2019-04-02T22:21:09Z

This comment was made automatically. If there is a problem contact ryanbrandenburg.

I've triaged the above build.

ryanbrandenburg · 2019-04-03T18:52:40Z

This comment was made automatically. If there is a problem contact ryanbrandenburg.

I've triaged the above build.

aspnet-hello · 2019-04-03T19:39:18Z

This comment was made automatically. If there is a problem contact ryanbrandenburg.

I've triaged the above build.

JunTaoLuo · 2019-04-04T17:48:12Z

src/ProjectTemplates/test/GrpcTemplateTest.cs

+
+namespace Templates.Test
+{
+    public class GrpcTemplateTest


@ryanbrandenburg we are going to be changing the template for gRPC pretty significantly since we'll be removing the client. We're going to need to update the test since our PRs will collide.

cc @JamesNK @shirhatti

ryanbrandenburg changed the title ~~Rybrande/templating links~~ Test all links in templates Mar 18, 2019

ryanbrandenburg commented Mar 18, 2019

View reviewed changes

src/ProjectTemplates/test/EmptyWebTemplateTest.cs Outdated Show resolved Hide resolved

ryanbrandenburg requested a review from javiercn March 18, 2019 19:49

Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Mar 18, 2019

ryanbrandenburg force-pushed the rybrande/TemplatingLinks branch from 86b9421 to bfd2d04 Compare March 21, 2019 21:47

javiercn reviewed Mar 25, 2019

View reviewed changes

src/ProjectTemplates/test/Helpers/AspNetProcess.cs Outdated Show resolved Hide resolved

ryanbrandenburg force-pushed the rybrande/TemplatingLinks branch from bfd2d04 to c02083b Compare March 26, 2019 16:28

pranavkm reviewed Mar 26, 2019

View reviewed changes

ryanbrandenburg requested a review from javiercn March 27, 2019 19:34

javiercn reviewed Mar 28, 2019

View reviewed changes

ryanbrandenburg force-pushed the rybrande/TemplatingLinks branch from a7f1c32 to afb8c22 Compare April 1, 2019 18:15

ryanbrandenburg requested review from analogrelay and Tratcher as code owners April 1, 2019 18:15

analogrelay removed their request for review April 1, 2019 19:17

ryanbrandenburg requested a review from javiercn April 2, 2019 18:13

javiercn reviewed Apr 2, 2019

View reviewed changes

javiercn approved these changes Apr 2, 2019

View reviewed changes

ryanbrandenburg force-pushed the rybrande/TemplatingLinks branch from afb8c22 to 7fcb33d Compare April 2, 2019 18:53

ryanbrandenburg added 9 commits April 3, 2019 12:55

Improve templating tests

9627009

Rebase

7767953

Condense url checks

1cff6a0

AddRazorSupportForMvc

8424edd

PR feedback

93e3904

Start on Grpc template test

bf6bd17

PR feedback

ad76c7f

Fix compile error

3840127

Fix baseline

a36cb35

ryanbrandenburg force-pushed the rybrande/TemplatingLinks branch from 726cd57 to a36cb35 Compare April 3, 2019 21:49

ryanbrandenburg added 2 commits April 3, 2019 15:51

Update baseline

fd26eaa

Remove extra file

e14f810

JunTaoLuo reviewed Apr 4, 2019

View reviewed changes

ryanbrandenburg merged commit fefffd7 into master Apr 4, 2019

ryanbrandenburg deleted the rybrande/TemplatingLinks branch April 4, 2019 18:31

This was referenced Apr 19, 2019

Add tests for Worker template #8754

Closed

Add tests for Worker template #7974

Closed

		@@ -23,6 +24,7 @@ public RazorPagesTemplateTest(ProjectFactoryFixture projectFactory, ITestOutputH

		public ITestOutputHelper Output { get; }

Test all links in templates #8628

Test all links in templates #8628

Uh oh!

Conversation

ryanbrandenburg commented Mar 18, 2019

Uh oh!

Uh oh!

ryanbrandenburg commented Mar 25, 2019

Uh oh!

Uh oh!

javiercn commented Mar 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanbrandenburg commented Mar 25, 2019

Uh oh!

pranavkm commented Mar 26, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javiercn left a comment

Choose a reason for hiding this comment

Uh oh!

ryanbrandenburg commented Apr 1, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javiercn left a comment

Choose a reason for hiding this comment

Uh oh!

ryanbrandenburg commented Apr 2, 2019

Uh oh!

aspnet-hello commented Apr 2, 2019

Uh oh!

ryanbrandenburg commented Apr 3, 2019

Uh oh!

aspnet-hello commented Apr 3, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

javiercn commented Mar 25, 2019 •

edited

Loading