Addressed code review comments.

Author: N. Taylor Mullen

src/Microsoft.AspNet.Mvc.TagHelpers/Properties/AssemblyInfo.cs

@@ -0,0 +1,6 @@
+// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System.Runtime.CompilerServices;
+
+[assembly: InternalsVisibleTo("Microsoft.AspNet.Mvc.TagHelpers.Test")]

src/Microsoft.AspNet.Mvc.TagHelpers/ValidationSummary.cs

@@ -14,7 +14,7 @@ public enum ValidationSummary
         None,
 
         /// <summary>
-        /// Validation summary with model-level errors only.
+        /// Validation summary with model-level errors only (excludes all property errors).
         /// </summary>
         ModelOnly,
 

src/Microsoft.AspNet.Mvc.TagHelpers/ValidationSummaryTagHelper.cs

@@ -9,27 +9,31 @@
 namespace Microsoft.AspNet.Mvc.TagHelpers
 {
     /// <summary>
-    /// <see cref="ITagHelper"/> implementation targeting &lt;div&gt; elements with <c>validation-summary</c> 
-    /// attributes.
-    /// </summary>S
+    /// <see cref="ITagHelper"/> implementation targeting &lt;div&gt; elements with a <c>validation-summary</c> 
+    /// attribute.
+    /// </summary>
     [TagName("div")]
     [ContentBehavior(ContentBehavior.Append)]
     public class ValidationSummaryTagHelper : TagHelper
     {
+        // Protected to ensure subclasses are correctly activated. Internal for ease of use when testing.
         [Activate]
-        private ViewContext ViewContext { get; set; }
+        protected internal ViewContext ViewContext { get; set; }
 
+        // Protected to ensure subclasses are correctly activated. Internal for ease of use when testing.
         [Activate]
-        private IHtmlGenerator Generator { get; set; }
+        protected internal IHtmlGenerator Generator { get; set; }
 
         // TODO: Change to ValidationSummary enum once https://github.com/aspnet/Razor/issues/196 has been completed.
         /// <summary>
-        /// Acceptable values are defined by the <see cref="ValidationSummary"/> enum.
+        /// If <c>All</c> or <c>ModelOnly</c>, appends a validation summary. Acceptable values are defined by the 
+        /// <see cref="ValidationSummary"/> enum.
         /// </summary>
         [HtmlAttributeName("validation-summary")]
         public string ValidationSummaryValue { get; set; }
 
         /// <inheritdoc />
+        /// Does nothing if <see cref="ValidationSummaryValue"/> is <c>null</c>, empty or "None".
         public override void Process(TagHelperContext context, TagHelperOutput output)
         {
             if (!string.IsNullOrEmpty(ValidationSummaryValue))
@@ -41,9 +45,9 @@ public override void Process(TagHelperContext context, TagHelperOutput output)
                         Resources.FormatValidationSummaryTagHelper_InvalidValidationSummaryValue(
                             "validation-summary",
                             ValidationSummaryValue,
-                            ValidationSummary.All.ToString(),
-                            ValidationSummary.ModelOnly.ToString(),
-                            ValidationSummary.None.ToString()));
+                            ValidationSummary.All,
+                            ValidationSummary.ModelOnly,
+                            ValidationSummary.None));
                 }
                 else if (validationSummaryValue == ValidationSummary.None)
                 {

test/Microsoft.AspNet.Mvc.TagHelpers.Test/ValidationSummaryTagHelperTest.cs

@@ -10,6 +10,7 @@
 using Microsoft.AspNet.Mvc.ModelBinding;
 using Microsoft.AspNet.Mvc.Razor;
 using Microsoft.AspNet.Mvc.Rendering;
+using Microsoft.AspNet.PipelineCore;
 using Microsoft.AspNet.Razor.Runtime.TagHelpers;
 using Microsoft.AspNet.Routing;
 using Moq;
@@ -41,8 +42,8 @@ public async Task ProcessAsync_GeneratesExpectedOutput()
             var htmlGenerator = new TestableHtmlGenerator(metadataProvider);
             Model model = null;
             var viewContext = TestableHtmlGenerator.GetViewContext(model, htmlGenerator, metadataProvider);
-            var activator = new DefaultTagHelperActivator();
-            activator.Activate(validationSummaryTagHelper, viewContext);
+            validationSummaryTagHelper.ViewContext = viewContext;
+            validationSummaryTagHelper.Generator = htmlGenerator;
 
             // Act
             await validationSummaryTagHelper.ProcessAsync(tagHelperContext, output);
@@ -76,8 +77,8 @@ public async Task ProcessAsync_CallsIntoGenerateValidationSummaryWithExpectedPar
                 .Setup(mock => mock.GenerateValidationSummary(expectedViewContext, true, null, null, null))
                 .Returns(new TagBuilder("div"))
                 .Verifiable();
-
-            SetViewContextAndGenerator(validationSummaryTagHelper, expectedViewContext, generator.Object);
+            validationSummaryTagHelper.ViewContext = expectedViewContext;
+            validationSummaryTagHelper.Generator = generator.Object;
 
             // Act & Assert
             await validationSummaryTagHelper.ProcessAsync(context: null, output: output);
@@ -107,29 +108,33 @@ public async Task ProcessAsync_MergesTagBuilderFromGenerateValidationSummary()
 
             tagBuilder.Attributes.Add("data-foo", "bar");
             tagBuilder.Attributes.Add("data-hello", "world");
+            tagBuilder.Attributes.Add("anything", "something");
 
-            var expectedViewContext = CreateViewContext();
             var generator = new Mock<IHtmlGenerator>(MockBehavior.Strict);
-            var setup = generator
-                .Setup(mock => mock.GenerateValidationSummary(It.IsAny<ViewContext>(),
+            generator
+                .Setup(mock => mock.GenerateValidationSummary(
+                    It.IsAny<ViewContext>(),
                     It.IsAny<bool>(),
                     It.IsAny<string>(),
                     It.IsAny<string>(),
                     It.IsAny<object>()))
                 .Returns(tagBuilder);
-
-            SetViewContextAndGenerator(validationSummaryTagHelper, expectedViewContext, generator.Object);
+            var viewContext = CreateViewContext();
+            validationSummaryTagHelper.ViewContext = viewContext;
+            validationSummaryTagHelper.Generator = generator.Object;
 
             // Act
             await validationSummaryTagHelper.ProcessAsync(context: null, output: output);
 
             // Assert
             Assert.Equal(output.TagName, "div");
-            Assert.Equal(2, output.Attributes.Count);
+            Assert.Equal(3, output.Attributes.Count);
             var attribute = Assert.Single(output.Attributes, kvp => kvp.Key.Equals("data-foo"));
             Assert.Equal("bar", attribute.Value);
             attribute = Assert.Single(output.Attributes, kvp => kvp.Key.Equals("data-hello"));
             Assert.Equal("world", attribute.Value);
+            attribute = Assert.Single(output.Attributes, kvp => kvp.Key.Equals("anything"));
+            Assert.Equal("something", attribute.Value);
             Assert.Equal("Content of validation summaryNew HTML", output.Content);
         }
 
@@ -148,10 +153,10 @@ public async Task ProcessAsync_DoesNothingIfNullOrEmptyValidationSummaryValue(st
                 attributes: new Dictionary<string, string>(),
                 content: "Content of validation message");
 
-            var expectedViewContext = CreateViewContext();
             var generator = new Mock<IHtmlGenerator>(MockBehavior.Strict);
-
-            SetViewContextAndGenerator(validationSummaryTagHelper, expectedViewContext, generator.Object);
+            var viewContext = CreateViewContext();
+            validationSummaryTagHelper.ViewContext = viewContext;
+            validationSummaryTagHelper.Generator = generator.Object;
 
             // Act
             await validationSummaryTagHelper.ProcessAsync(context: null, output: output);
@@ -183,18 +188,19 @@ public async Task ProcessAsync_GeneratesValidationSummaryWhenNotNone_IgnoresCase
                 InnerHtml = "New HTML"
             };
 
-            var expectedViewContext = CreateViewContext();
-            var generator = new Mock<IHtmlGenerator>(MockBehavior.Loose);
+            var generator = new Mock<IHtmlGenerator>();
             generator
-                .Setup(mock => mock.GenerateValidationSummary(It.IsAny<ViewContext>(),
+                .Setup(mock => mock.GenerateValidationSummary(
+                    It.IsAny<ViewContext>(),
                     It.IsAny<bool>(),
                     It.IsAny<string>(),
                     It.IsAny<string>(),
                     It.IsAny<object>()))
                 .Returns(tagBuilder)
                 .Verifiable();
-
-            SetViewContextAndGenerator(validationSummaryTagHelper, expectedViewContext, generator.Object);
+            var viewContext = CreateViewContext();
+            validationSummaryTagHelper.ViewContext = viewContext;
+            validationSummaryTagHelper.Generator = generator.Object;
 
             // Act
             await validationSummaryTagHelper.ProcessAsync(context: null, output: output);
@@ -225,14 +231,7 @@ public async Task ProcessAsync_DoesNotGenerateValidationSummaryWhenNone_IgnoresC
                 InnerHtml = "New HTML"
             };
 
-            var generator = new Mock<IHtmlGenerator>(MockBehavior.Loose);
-            generator
-                .Setup(mock => mock.GenerateValidationSummary(It.IsAny<ViewContext>(),
-                    It.IsAny<bool>(),
-                    It.IsAny<string>(),
-                    It.IsAny<string>(),
-                    It.IsAny<object>()))
-                .Throws(new Exception("Shouldn't be called."));
+            var generator = new Mock<IHtmlGenerator>(MockBehavior.Strict);
 
             // Act
             await validationSummaryTagHelper.ProcessAsync(context: null, output: output);
@@ -260,10 +259,8 @@ public async Task ProcessAsync_ThrowsWhenInvalidValidationSummaryValue()
                                   "'All', 'ModelOnly' and 'None'.";
 
             // Act
-            var ex = await Assert.ThrowsAsync<InvalidOperationException>(async () =>
-            {
-                await validationSummaryTagHelper.ProcessAsync(context: null, output: output);
-            });
+            var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => 
+                validationSummaryTagHelper.ProcessAsync(context: null, output: output));
 
             // Assert
             Assert.Equal(expectedMessage, ex.Message);
@@ -272,31 +269,18 @@ public async Task ProcessAsync_ThrowsWhenInvalidValidationSummaryValue()
         private static ViewContext CreateViewContext()
         {
             var actionContext = new ActionContext(
-                new Mock<HttpContext>().Object,
+                new DefaultHttpContext(),
                 new RouteData(),
                 new ActionDescriptor());
 
             return new ViewContext(
                 actionContext,
                 Mock.Of<IView>(),
                 new ViewDataDictionary(
-                    new DataAnnotationsModelMetadataProvider()),
-                new StringWriter());
+                    new EmptyModelMetadataProvider()),
+                TextWriter.Null);
         }
 
-        private static void SetViewContextAndGenerator(ITagHelper tagHelper,
-                                                       ViewContext viewContext,
-                                                       IHtmlGenerator generator)
-        {
-            var tagHelperType = tagHelper.GetType();
-
-            tagHelperType.GetProperty("ViewContext", BindingFlags.NonPublic | BindingFlags.Instance)
-                         .SetValue(tagHelper, viewContext);
-            tagHelperType.GetProperty("Generator", BindingFlags.NonPublic | BindingFlags.Instance)
-                         .SetValue(tagHelper, generator);
-        }
-
-
         private class Model
         {
             public string Text { get; set; }