From fb9cbc0c9df01fba6ec7e4843c3ae9dc81734612 Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Sat, 15 Apr 2023 13:27:34 +0200 Subject: [PATCH 01/11] Rename variable, format expression --- .../Extensions/ScreenshotUITestContextExtensions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs index fd11de278..f69c04985 100644 --- a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs @@ -70,8 +70,8 @@ public static Image TakeFullPageScreenshot(this UITestContext context) var height = images.Keys.Sum( position => position.Y % viewportSize.Height == 0 - ? viewportSize.Height - : (position.Y + viewportSize.Height) % viewportSize.Height); + ? viewportSize.Height + : (position.Y + viewportSize.Height) % viewportSize.Height); var screenshot = new SixLabors.ImageSharp.Image(viewportSize.Width, height); @@ -119,8 +119,8 @@ public static Image TakeElementScreenshot(this UITestContext context, IWebElemen + $"{elementAbsoluteSize.Height.ToTechnicalString()}."); } - var bounds = new Rectangle(element.Location.X, element.Location.Y, element.Size.Width, element.Size.Height); - return screenshot.Clone(ctx => ctx.Crop(bounds)); + var cropRectangle = new Rectangle(elementLocation.X, elementLocation.Y, elementSize.Width, elementSize.Height); + return screenshot.Clone(ctx => ctx.Crop(cropRectangle)); } /// From 08a797e722ff63f980069dec9b7cbd2e4c8ad424 Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Sat, 15 Apr 2023 13:33:32 +0200 Subject: [PATCH 02/11] Add 1px tolerance for calculated page size due to possible rounding error --- .../ScreenshotUITestContextExtensions.cs | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs index f69c04985..769cd8bbc 100644 --- a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs @@ -105,18 +105,32 @@ public static Image TakeElementScreenshot(this UITestContext context, IWebElemen { using var screenshot = context.TakeFullPageScreenshot(); - var elementAbsoluteSize = new Size( - element.Location.X + element.Size.Width, - element.Location.Y + element.Size.Height); + var elementLocation = element.Location; + var elementSize = element.Size; - if (elementAbsoluteSize.Width > screenshot.Width || elementAbsoluteSize.Height > screenshot.Height) + var expectedPageSize = new Size(elementLocation.X + elementSize.Width, elementLocation.Y + elementSize.Height); + + var widthDifference = expectedPageSize.Width - screenshot.Width; + var heightDifference = expectedPageSize.Height - screenshot.Height; + + // A difference of 1px can occur when both element.Location and element.Size were rounded to the next + // integer from exactly 0.5px, e.g. 212.5px + 287.5px = 500px in the browser, but due to both Size and Point + // using int for their coordinates, this will become 213px + 288px = 501px, which will be caught as an error + // here. In that case, we keep the elementSize as is, but reduce the Location coordinate by 1 so that cropping + // the full page screenshot to the desired region will not fail due to too large dimensions. + if (widthDifference <= 1 && heightDifference <= 1) + { + elementLocation.X -= widthDifference; + elementLocation.Y -= heightDifference; + } + else { throw new InvalidOperationException( "The captured screenshot size is smaller then the size required by the selected element. This can occur" + " if there was an unsuccessful scrolling operation while capturing page parts." - + $"Captured size: {screenshot.Width.ToTechnicalString()} x {screenshot.Height.ToTechnicalString()}. " - + $"Required size: {elementAbsoluteSize.Width.ToTechnicalString()} x " - + $"{elementAbsoluteSize.Height.ToTechnicalString()}."); + + $" Captured size: {screenshot.Width.ToTechnicalString()} x {screenshot.Height.ToTechnicalString()}." + + $" Expected size: {expectedPageSize.Width.ToTechnicalString()} x" + + $" {expectedPageSize.Height.ToTechnicalString()}."); } var cropRectangle = new Rectangle(elementLocation.X, elementLocation.Y, elementSize.Width, elementSize.Height); From 34ed51ea51d7529d49675fe749dc0a7c13cfd106 Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Sat, 15 Apr 2023 13:42:49 +0200 Subject: [PATCH 03/11] Simplify pattern matching, improve formatting --- .../Services/OrchardCoreUITestExecutorConfiguration.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs b/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs index 45d9a3b1a..ba83ece63 100644 --- a/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs +++ b/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs @@ -53,10 +53,10 @@ public class OrchardCoreUITestExecutorConfiguration /// /// public int MaxParallelTests { get; set; } = - TestConfigurationManager.GetIntConfiguration( - $"{nameof(OrchardCoreUITestExecutorConfiguration)}:{nameof(MaxParallelTests)}") is not { } intValue || intValue == 0 - ? Environment.ProcessorCount - : intValue; + TestConfigurationManager.GetIntConfiguration( + $"{nameof(OrchardCoreUITestExecutorConfiguration)}:{nameof(MaxParallelTests)}") is { } intValue and > 0 + ? intValue + : Environment.ProcessorCount; public Func AssertAppLogsAsync { get; set; } = AssertAppLogsCanContainWarningsAsync; public Action> AssertBrowserLog { get; set; } = AssertBrowserLogIsEmpty; From e211e1c6db86f827e91a7bf418e2bd713473a6b0 Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Sat, 15 Apr 2023 13:49:37 +0200 Subject: [PATCH 04/11] Move static fields and read-only properties to beginning of file --- .../OrchardCoreUITestExecutorConfiguration.cs | 68 +++++++++---------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs b/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs index ba83ece63..12f32776a 100644 --- a/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs +++ b/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs @@ -21,6 +21,40 @@ public enum Browser public class OrchardCoreUITestExecutorConfiguration { + public static readonly Func AssertAppLogsAreEmptyAsync = app => app.LogsShouldBeEmptyAsync(); + + public static readonly Func AssertAppLogsCanContainWarningsAsync = + app => app.LogsShouldBeEmptyAsync(canContainWarnings: true); + + public static readonly Action> AssertBrowserLogIsEmpty = + logEntries => logEntries.ShouldNotContain( + logEntry => IsValidBrowserLogEntry(logEntry), + logEntries.Where(IsValidBrowserLogEntry).ToFormattedString()); + + public static readonly Func IsValidBrowserLogEntry = + logEntry => + logEntry.Level >= LogLevel.Warning && + // HTML imports are somehow used by Selenium or something but this deprecation notice is always there for every + // page. + !logEntry.Message.ContainsOrdinalIgnoreCase("HTML Imports is deprecated") && + // The 404 is because of how browsers automatically request /favicon.ico even if a favicon is declared to be + // under a different URL. + !logEntry.IsNotFoundLogEntry("/favicon.ico"); + + /// + /// Gets the global events available during UI test execution. + /// + public UITestExecutionEvents Events { get; } = new(); + + /// + /// Gets a dictionary storing some custom configuration data. + /// + [SuppressMessage( + "Design", + "MA0016:Prefer return collection abstraction instead of implementation", + Justification = "Deliberately modifiable by consumer code.")] + public Dictionary CustomConfiguration { get; } = new(); + public BrowserConfiguration BrowserConfiguration { get; set; } = new(); public TimeoutConfiguration TimeoutConfiguration { get; set; } = TimeoutConfiguration.Default; public AtataConfiguration AtataConfiguration { get; set; } = new(); @@ -136,20 +170,6 @@ public class OrchardCoreUITestExecutorConfiguration /// public ShortcutsConfiguration ShortcutsConfiguration { get; set; } = new(); - /// - /// Gets the global events available during UI test execution. - /// - public UITestExecutionEvents Events { get; } = new(); - - /// - /// Gets a dictionary storing some custom configuration data. - /// - [SuppressMessage( - "Design", - "MA0016:Prefer return collection abstraction instead of implementation", - Justification = "Deliberately modifiable by consumer code.")] - public Dictionary CustomConfiguration { get; } = new(); - public async Task AssertAppLogsMaybeAsync(IWebApplicationInstance instance, Action log) { if (instance == null || AssertAppLogsAsync == null) return; @@ -183,24 +203,4 @@ public void AssertBrowserLogMaybe(IList browserLogs, Action lo throw; } } - - public static readonly Func AssertAppLogsAreEmptyAsync = app => app.LogsShouldBeEmptyAsync(); - - public static readonly Func AssertAppLogsCanContainWarningsAsync = - app => app.LogsShouldBeEmptyAsync(canContainWarnings: true); - - public static readonly Action> AssertBrowserLogIsEmpty = - // HTML imports are somehow used by Selenium or something but this deprecation notice is always there for every - // page. - logEntries => logEntries.ShouldNotContain( - logEntry => IsValidBrowserLogEntry(logEntry), - logEntries.Where(IsValidBrowserLogEntry).ToFormattedString()); - - public static readonly Func IsValidBrowserLogEntry = - logEntry => - logEntry.Level >= LogLevel.Warning && - !logEntry.Message.ContainsOrdinalIgnoreCase("HTML Imports is deprecated") && - // The 404 is because of how browsers automatically request /favicon.ico even if a favicon is declared to be - // under a different URL. - !logEntry.IsNotFoundLogEntry("/favicon.ico"); } From c4048f303696c22b7128832cf6d48d736f644887 Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Sat, 15 Apr 2023 22:32:18 +0200 Subject: [PATCH 05/11] Fix logic error in difference comparison --- .../ScreenshotUITestContextExtensions.cs | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs index 769cd8bbc..f4362ba22 100644 --- a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs @@ -113,24 +113,27 @@ public static Image TakeElementScreenshot(this UITestContext context, IWebElemen var widthDifference = expectedPageSize.Width - screenshot.Width; var heightDifference = expectedPageSize.Height - screenshot.Height; - // A difference of 1px can occur when both element.Location and element.Size were rounded to the next - // integer from exactly 0.5px, e.g. 212.5px + 287.5px = 500px in the browser, but due to both Size and Point - // using int for their coordinates, this will become 213px + 288px = 501px, which will be caught as an error - // here. In that case, we keep the elementSize as is, but reduce the Location coordinate by 1 so that cropping - // the full page screenshot to the desired region will not fail due to too large dimensions. - if (widthDifference <= 1 && heightDifference <= 1) + if (widthDifference > 0 || heightDifference > 0) { - elementLocation.X -= widthDifference; - elementLocation.Y -= heightDifference; - } - else - { - throw new InvalidOperationException( - "The captured screenshot size is smaller then the size required by the selected element. This can occur" - + " if there was an unsuccessful scrolling operation while capturing page parts." - + $" Captured size: {screenshot.Width.ToTechnicalString()} x {screenshot.Height.ToTechnicalString()}." - + $" Expected size: {expectedPageSize.Width.ToTechnicalString()} x" - + $" {expectedPageSize.Height.ToTechnicalString()}."); + // A difference of 1px can occur when both element.Location and element.Size were rounded to the next + // integer from exactly 0.5px, e.g. 212.5px + 287.5px = 500px in the browser, but due to both Size and Point + // using int for their coordinates, this will become 213px + 288px = 501px, which will be caught as an error + // here. In that case, we keep the elementSize as is, but reduce the Location coordinate by 1 so that cropping + // the full page screenshot to the desired region will not fail due to too large dimensions. + if (widthDifference <= 1 && heightDifference <= 1) + { + elementLocation.X -= widthDifference; + elementLocation.Y -= heightDifference; + } + else + { + throw new InvalidOperationException( + "The captured screenshot size is smaller then the size required by the selected element. This can occur" + + " if there was an unsuccessful scrolling operation while capturing page parts." + + $" Captured size: {screenshot.Width.ToTechnicalString()} x {screenshot.Height.ToTechnicalString()}." + + $" Expected size: {expectedPageSize.Width.ToTechnicalString()} x" + + $" {expectedPageSize.Height.ToTechnicalString()}."); + } } var cropRectangle = new Rectangle(elementLocation.X, elementLocation.Y, elementSize.Width, elementSize.Height); From ceee1013afff06f55cb88d89a26b9ff7f8ec8d61 Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Sat, 15 Apr 2023 22:49:38 +0200 Subject: [PATCH 06/11] Improve variable naming and error message formatting --- .../ScreenshotUITestContextExtensions.cs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs index f4362ba22..b36cd11e3 100644 --- a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs @@ -108,10 +108,10 @@ public static Image TakeElementScreenshot(this UITestContext context, IWebElemen var elementLocation = element.Location; var elementSize = element.Size; - var expectedPageSize = new Size(elementLocation.X + elementSize.Width, elementLocation.Y + elementSize.Height); + var expectedSize = new Size(elementLocation.X + elementSize.Width, elementLocation.Y + elementSize.Height); - var widthDifference = expectedPageSize.Width - screenshot.Width; - var heightDifference = expectedPageSize.Height - screenshot.Height; + var widthDifference = expectedSize.Width - screenshot.Width; + var heightDifference = expectedSize.Height - screenshot.Height; if (widthDifference > 0 || heightDifference > 0) { @@ -128,11 +128,9 @@ public static Image TakeElementScreenshot(this UITestContext context, IWebElemen else { throw new InvalidOperationException( - "The captured screenshot size is smaller then the size required by the selected element. This can occur" - + " if there was an unsuccessful scrolling operation while capturing page parts." - + $" Captured size: {screenshot.Width.ToTechnicalString()} x {screenshot.Height.ToTechnicalString()}." - + $" Expected size: {expectedPageSize.Width.ToTechnicalString()} x" - + $" {expectedPageSize.Height.ToTechnicalString()}."); + "The captured screenshot size is smaller then the size required by the selected element. This can" + + " occur if there was an unsuccessful scrolling operation while capturing page parts." + + $" Captured size: {AsDimensions(screenshot)}. Expected size: {AsDimensions(expectedSize)}."); } } @@ -145,4 +143,10 @@ public static Image TakeElementScreenshot(this UITestContext context, IWebElemen /// public static Image TakeElementScreenshot(this UITestContext context, By elementSelector) => context.TakeElementScreenshot(context.Get(elementSelector)); + + private static string AsDimensions(IImageInfo image) => + $"{image.Width.ToTechnicalString()} x {image.Height.ToTechnicalString()}"; + + private static string AsDimensions(Size size) => + $"{size.Width.ToTechnicalString()} x {size.Height.ToTechnicalString()}"; } From d3c0f768edf75d85ffccec794811f832304f9a2f Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Mon, 17 Apr 2023 11:19:07 +0200 Subject: [PATCH 07/11] Wrap long comment line --- .../Extensions/ScreenshotUITestContextExtensions.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs index b36cd11e3..b8981a5e9 100644 --- a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs @@ -118,8 +118,8 @@ public static Image TakeElementScreenshot(this UITestContext context, IWebElemen // A difference of 1px can occur when both element.Location and element.Size were rounded to the next // integer from exactly 0.5px, e.g. 212.5px + 287.5px = 500px in the browser, but due to both Size and Point // using int for their coordinates, this will become 213px + 288px = 501px, which will be caught as an error - // here. In that case, we keep the elementSize as is, but reduce the Location coordinate by 1 so that cropping - // the full page screenshot to the desired region will not fail due to too large dimensions. + // here. In that case, we keep the elementSize as is, but reduce the Location coordinate by 1 so that + // cropping the full page screenshot to the desired region will not fail due to too large dimensions. if (widthDifference <= 1 && heightDifference <= 1) { elementLocation.X -= widthDifference; From 45c59ecddb3672cb03310cf2a48c167160c22db6 Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Mon, 17 Apr 2023 11:21:32 +0200 Subject: [PATCH 08/11] Improve parameter name --- Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs index b8981a5e9..77adc3298 100644 --- a/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/Extensions/ScreenshotUITestContextExtensions.cs @@ -135,7 +135,7 @@ public static Image TakeElementScreenshot(this UITestContext context, IWebElemen } var cropRectangle = new Rectangle(elementLocation.X, elementLocation.Y, elementSize.Width, elementSize.Height); - return screenshot.Clone(ctx => ctx.Crop(cropRectangle)); + return screenshot.Clone(image => image.Crop(cropRectangle)); } /// From 509a26bb25ae6467d1d4736a9efdb160a632b521 Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Mon, 17 Apr 2023 11:27:52 +0200 Subject: [PATCH 09/11] Wrap long lines --- .../Services/OrchardCoreUITestExecutorConfiguration.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs b/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs index 12f32776a..9aa85452c 100644 --- a/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs +++ b/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs @@ -21,7 +21,8 @@ public enum Browser public class OrchardCoreUITestExecutorConfiguration { - public static readonly Func AssertAppLogsAreEmptyAsync = app => app.LogsShouldBeEmptyAsync(); + public static readonly Func AssertAppLogsAreEmptyAsync = app => + app.LogsShouldBeEmptyAsync(); public static readonly Func AssertAppLogsCanContainWarningsAsync = app => app.LogsShouldBeEmptyAsync(canContainWarnings: true); @@ -34,8 +35,8 @@ public class OrchardCoreUITestExecutorConfiguration public static readonly Func IsValidBrowserLogEntry = logEntry => logEntry.Level >= LogLevel.Warning && - // HTML imports are somehow used by Selenium or something but this deprecation notice is always there for every - // page. + // HTML imports are somehow used by Selenium or something but this deprecation notice is always there for + // every page. !logEntry.Message.ContainsOrdinalIgnoreCase("HTML Imports is deprecated") && // The 404 is because of how browsers automatically request /favicon.ico even if a favicon is declared to be // under a different URL. From f900731f861c4d2c7952a7d625d3654a5350bca1 Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Mon, 17 Apr 2023 12:09:52 +0200 Subject: [PATCH 10/11] Use ctor with optional parameter --- .../VisualVerificationBaselineImageNotFoundException.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Lombiq.Tests.UI/Exceptions/VisualVerificationBaselineImageNotFoundException.cs b/Lombiq.Tests.UI/Exceptions/VisualVerificationBaselineImageNotFoundException.cs index a72182f96..72b20fe3b 100644 --- a/Lombiq.Tests.UI/Exceptions/VisualVerificationBaselineImageNotFoundException.cs +++ b/Lombiq.Tests.UI/Exceptions/VisualVerificationBaselineImageNotFoundException.cs @@ -7,12 +7,7 @@ namespace Lombiq.Tests.UI.Exceptions; public class VisualVerificationBaselineImageNotFoundException : Exception #pragma warning restore CA1032 // Implement standard exception constructors { - public VisualVerificationBaselineImageNotFoundException(string path) - : this(path, innerException: null) - { - } - - public VisualVerificationBaselineImageNotFoundException(string path, Exception innerException) + public VisualVerificationBaselineImageNotFoundException(string path, Exception innerException = null) : base( $"Baseline image file not found, thus it was created automatically under the path {path}." + " Please set its \"Build action\" to \"Embedded resource\" if you want to deploy a self-contained" From 4e84aafa471c116b65f511f697d715c8e79288cd Mon Sep 17 00:00:00 2001 From: Oliver Friedrich Date: Mon, 17 Apr 2023 12:13:36 +0200 Subject: [PATCH 11/11] Try removing the nodes from Samples project - shouldn't be needed, since we don't package this project for reuse --- Lombiq.Tests.UI.Samples/Lombiq.Tests.UI.Samples.csproj | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Lombiq.Tests.UI.Samples/Lombiq.Tests.UI.Samples.csproj b/Lombiq.Tests.UI.Samples/Lombiq.Tests.UI.Samples.csproj index 344ea5d6a..7ef093144 100644 --- a/Lombiq.Tests.UI.Samples/Lombiq.Tests.UI.Samples.csproj +++ b/Lombiq.Tests.UI.Samples/Lombiq.Tests.UI.Samples.csproj @@ -12,7 +12,7 @@ - + Always @@ -21,14 +21,6 @@ PreserveNewest - - - - - - - -