-
Notifications
You must be signed in to change notification settings - Fork 6
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
OSOE-353: Add duplicate SQL query detector in Lombiq.UITestingToolbox #216
Open
dministro
wants to merge
70
commits into
dev
Choose a base branch
from
issue/OSOE-353
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
70 commits
Select commit
Hold shift + click to select a range
f224aff
Adding counter infrastructure
dministro d87e325
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 34b538b
Using interfaces
dministro 7574e9a
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro beef486
Implementing required exception constructors
dministro 6c3602f
Adjusting tests
dministro 06f8eaa
Fixing typo
dministro 3cd03e3
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 8335d59
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 914c291
Update Lombiq.Tests.UI/Services/CounterDataCollector.cs
dministro 1100237
Update Lombiq.Tests.UI/Services/Counters/CounterProbeBase.cs
dministro df3c3c2
Update Lombiq.Tests.UI/Services/Counters/ICounterValue.cs
dministro 1a0fd1a
Update Lombiq.Tests.UI/Services/Counters/NavigationProbe.cs
dministro 8e61c22
Update Lombiq.Tests.UI.Samples/Tests/BasicOrchardFeaturesTests.cs
dministro a20311c
Update Lombiq.Tests.UI/Services/CounterConfiguration.cs
dministro f033443
Merge branch 'issue/OSOE-353' of https://github.com/Lombiq/UI-Testing…
dministro cac8255
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 169fa26
Adding SessionProbe
dministro cc941b0
Adding better exclusion and defaults
dministro 8e854fc
Addressing "Add docs to the properties."
dministro dff34c3
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 33fd4fe
Fixes after merge
dministro f0d8fc0
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 0a8189c
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro d0a088f
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 5d32c2f
Fixing analyzer errors
dministro 448a46e
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 0e1cdfe
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro eb8adc9
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro b416eb5
Fixes after merge
dministro bc91080
Fixing possible thread safety violation
dministro b3e0600
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 278fdcc
Addressing "S4457: Split this method into two..." analyzer error
dministro 2fd58bd
Adding per url threshold configuration support
dministro 98a596e
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 8c05e71
Adding per page configuration implementation
dministro 097d091
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 030c692
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 406e6ed
Fixing analyzer violations
dministro 6bf2917
Adding ProbedSqliteConnection
dministro acf440f
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 3342c9d
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 126c97b
Improving log and exception message
dministro 82c9090
Improving logs
dministro f50c13c
Throwing session probe exception in test context
dministro 8a0b33c
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro de38ccd
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 8561fac
Fixing test
dministro fb9be54
Adding comment
dministro 4935a11
Fix spelling
dministro c2906c7
Apply suggestions from code review
dministro 8c58730
Apply suggestions from code review
dministro c953c8c
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 3343bd8
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 4f334fe
Fixes after merge
dministro 027486d
Fixing analyzer violations
dministro fa3a95c
Fixing analyzer violations again
dministro 0e4f177
Adding custom type for storing db command parameters in configuration…
dministro 849553f
Allowing to enable/disable counter subsystem by configuration
dministro 9203e36
Adding a bit more information to CounterThresholdException message
dministro a52cff1
Removing unnecessary Should.NotThrowAsync()
dministro 30b4997
Adding test to demonstrate the scenario when the counter thresholds a…
dministro 2d5abae
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 1b500d8
Removing unnecessary configuration
dministro bca3add
Renaming `PageLoadProbe` -> `RequestProbe` and `PageLoadProbeMiddlewa…
dministro a1531d5
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 71663b9
Adding more comments
dministro 1dea988
Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…
dministro 2c7e741
Spelling
dministro a6f685c
Fixing analyzer warnings
dministro File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
115 changes: 115 additions & 0 deletions
115
Lombiq.Tests.UI.Samples/Tests/DuplicatedSqlQueryDetectorTests.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
using Lombiq.Tests.UI.Extensions; | ||
using Lombiq.Tests.UI.Services; | ||
using Lombiq.Tests.UI.Services.Counters.Configuration; | ||
using Shouldly; | ||
using System; | ||
using System.Threading.Tasks; | ||
using Xunit; | ||
using Xunit.Abstractions; | ||
|
||
namespace Lombiq.Tests.UI.Samples.Tests; | ||
|
||
// Some times you may want to detect duplicated SQL queries. This can be useful if you want to make sure that your code | ||
// does not execute the same query multiple times, wasting time and computing resources. | ||
public class DuplicatedSqlQueryDetectorTests : UITestBase | ||
{ | ||
public DuplicatedSqlQueryDetectorTests(ITestOutputHelper testOutputHelper) | ||
: base(testOutputHelper) | ||
{ | ||
} | ||
|
||
// This test will fail because the app will read the same command result more times than the configured threshold | ||
// during the Admin page rendering. | ||
[Fact] | ||
public Task PageWithTooManyReadsOnDbDataReaderShouldThrow() => | ||
Should.ThrowAsync<AggregateException>(() => | ||
ExecuteTestAfterSetupAsync( | ||
context => context.SignInDirectlyAndGoToDashboardAsync(), | ||
configuration => ConfigureAsync( | ||
configuration, | ||
commandExcludingParametersThreshold: 3, | ||
commandIncludingParametersThreshold: 2, | ||
readerReadThreshold: 0))); | ||
|
||
// This test will fail because the app will execute the same SQL query having same parameter set more times than | ||
// expected during the Admin page rendering. | ||
[Fact] | ||
public Task PageWithTooManySqlQueriesExecutedHavingTheSameParameterSetShouldThrow() => | ||
Should.ThrowAsync<AggregateException>(() => | ||
ExecuteTestAfterSetupAsync( | ||
context => context.SignInDirectlyAndGoToDashboardAsync(), | ||
configuration => ConfigureAsync( | ||
configuration, | ||
commandExcludingParametersThreshold: 3, | ||
commandIncludingParametersThreshold: 0, | ||
readerReadThreshold: 2))); | ||
|
||
// This test will fail because the app will execute the same SQL query more times than expected during the Admin | ||
// page rendering. | ||
[Fact] | ||
public Task PageWithTooManySqlQueriesExecutedShouldThrow() => | ||
Should.ThrowAsync<AggregateException>(() => | ||
ExecuteTestAfterSetupAsync( | ||
context => context.SignInDirectlyAndGoToDashboardAsync(), | ||
configuration => ConfigureAsync( | ||
configuration, | ||
commandExcludingParametersThreshold: 0, | ||
commandIncludingParametersThreshold: 2, | ||
readerReadThreshold: 2))); | ||
|
||
// This test will pass because not any of the Admin page was loaded where the SQL queries are under monitoring. | ||
[Fact] | ||
public Task PageWithoutDuplicatedSqlQueriesShouldPass() => | ||
ExecuteTestAfterSetupAsync( | ||
context => context.GoToHomePageAsync(onlyIfNotAlreadyThere: false), | ||
configuration => ConfigureAsync(configuration)); | ||
|
||
// This test will pass because counter thresholds are exactly matching with the counter values captured during | ||
// navigating to the Admin dashboard page. | ||
[Fact] | ||
public Task PageWithMatchingCounterThresholdsShouldPass() => | ||
ExecuteTestAfterSetupAsync( | ||
context => context.SignInDirectlyAndGoToDashboardAsync(), | ||
configuration => ConfigureAsync( | ||
configuration, | ||
commandExcludingParametersThreshold: 3, | ||
commandIncludingParametersThreshold: 2, | ||
readerReadThreshold: 2)); | ||
|
||
// We configure the test to throw an exception if a certain counter threshold is exceeded, but only in case of Admin | ||
// pages. | ||
private static Task ConfigureAsync( | ||
OrchardCoreUITestExecutorConfiguration configuration, | ||
int commandExcludingParametersThreshold = 0, | ||
int commandIncludingParametersThreshold = 0, | ||
int readerReadThreshold = 0) | ||
{ | ||
// The test is guaranteed to fail so we don't want to retry it needlessly. | ||
configuration.MaxRetryCount = 0; | ||
|
||
var adminCounterConfiguration = new CounterConfiguration | ||
{ | ||
ExcludeFilter = OrchardCoreUITestExecutorConfiguration.DefaultCounterExcludeFilter, | ||
SessionThreshold = | ||
{ | ||
// Let's enable and configure the counter thresholds for ORM sessions. | ||
IsEnabled = true, | ||
DbCommandExcludingParametersExecutionThreshold = commandExcludingParametersThreshold, | ||
DbCommandIncludingParametersExecutionCountThreshold = commandIncludingParametersThreshold, | ||
DbReaderReadThreshold = readerReadThreshold, | ||
}, | ||
}; | ||
|
||
// Apply the configuration to the Admin pages only. | ||
configuration.CounterConfiguration.AfterSetup.Add( | ||
new RelativeUrlConfigurationKey(new Uri("/Admin", UriKind.Relative), exactMatch: false), | ||
adminCounterConfiguration); | ||
|
||
// Enable the counter subsystem. | ||
configuration.CounterConfiguration.IsEnabled = true; | ||
|
||
return Task.CompletedTask; | ||
} | ||
} | ||
|
||
// END OF TRAINING SECTION: Duplicated SQL query detector. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
using Lombiq.Tests.UI.Services.Counters; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Text; | ||
|
||
namespace Lombiq.Tests.UI.Exceptions; | ||
|
||
public class CounterThresholdException : Exception | ||
{ | ||
public CounterThresholdException() | ||
{ | ||
} | ||
|
||
public CounterThresholdException(string message) | ||
: this(probe: null, counter: null, value: null, message) | ||
{ | ||
} | ||
|
||
public CounterThresholdException(string message, Exception innerException) | ||
: this(probe: null, counter: null, value: null, message, innerException) | ||
{ | ||
} | ||
|
||
public CounterThresholdException( | ||
ICounterProbe probe, | ||
ICounterKey counter, | ||
ICounterValue value) | ||
: this(probe, counter, value, message: null, innerException: null) | ||
{ | ||
} | ||
|
||
public CounterThresholdException( | ||
ICounterProbe probe, | ||
ICounterKey counter, | ||
ICounterValue value, | ||
string message) | ||
: this(probe, counter, value, message, innerException: null) | ||
{ | ||
} | ||
|
||
public CounterThresholdException( | ||
ICounterProbe probe, | ||
ICounterKey counter, | ||
ICounterValue value, | ||
string message, | ||
Exception innerException) | ||
: base(FormatMessage(probe, counter, value, message), innerException) | ||
{ | ||
} | ||
|
||
private static string FormatMessage( | ||
ICounterProbe probe, | ||
ICounterKey counter, | ||
ICounterValue value, | ||
string message) | ||
{ | ||
var builder = new StringBuilder() | ||
.AppendLine() | ||
.AppendLine("A counter value has crossed the configured threshold level. Details:"); | ||
|
||
if (probe is not null) builder.AppendLine(probe.DumpHeadline()); | ||
counter?.Dump().ForEach(line => builder.AppendLine(line)); | ||
value?.Dump().ForEach(line => builder.AppendLine(line)); | ||
if (!string.IsNullOrEmpty(message)) builder.AppendLine(message); | ||
|
||
return builder.ToString(); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
using Lombiq.Tests.UI.SecurityScanning; | ||
using Lombiq.Tests.UI.Services; | ||
|
||
namespace Lombiq.Tests.UI.Models; | ||
|
||
internal sealed record UITestContextParameters | ||
{ | ||
public string Id { get; init; } | ||
public UITestManifest TestManifest { get; init; } | ||
public OrchardCoreUITestExecutorConfiguration Configuration { get; init; } | ||
public IWebApplicationInstance Application { get; init; } | ||
public AtataScope Scope { get; init; } | ||
public RunningContextContainer RunningContextContainer { get; init; } | ||
public ZapManager ZapManager { get; init; } | ||
public CounterDataCollector CounterDataCollector { get; init; } | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
using Lombiq.Tests.UI.Services.Counters; | ||
using System; | ||
using System.Collections.Concurrent; | ||
using System.Collections.Generic; | ||
using System.Linq; | ||
using Xunit.Abstractions; | ||
|
||
namespace Lombiq.Tests.UI.Services; | ||
|
||
public sealed class CounterDataCollector : CounterProbeBase, ICounterDataCollector | ||
{ | ||
private readonly ITestOutputHelper _testOutputHelper; | ||
private readonly ConcurrentBag<ICounterProbe> _probes = []; | ||
private readonly ConcurrentBag<Exception> _postponedCounterExceptions = []; | ||
public override bool IsAttached => true; | ||
public Action<ICounterDataCollector, ICounterProbe> AssertCounterData { get; set; } | ||
public string Phase { get; set; } | ||
public bool IsEnabled { get; set; } | ||
|
||
public CounterDataCollector(ITestOutputHelper testOutputHelper) => | ||
_testOutputHelper = testOutputHelper; | ||
|
||
public void AttachProbe(ICounterProbe probe) | ||
{ | ||
if (!IsEnabled) | ||
{ | ||
return; | ||
} | ||
|
||
probe.CaptureCompleted = ProbeCaptureCompleted; | ||
_probes.Add(probe); | ||
} | ||
|
||
public void Reset() | ||
{ | ||
_probes.Clear(); | ||
_postponedCounterExceptions.Clear(); | ||
Clear(); | ||
} | ||
|
||
public override void Increment(ICounterKey counter) | ||
{ | ||
if (!IsEnabled) | ||
{ | ||
return; | ||
} | ||
|
||
_probes.Where(probe => probe.IsAttached) | ||
.ForEach(probe => probe.Increment(counter)); | ||
base.Increment(counter); | ||
} | ||
|
||
public override string DumpHeadline() => $"{nameof(CounterDataCollector)}, Phase = {Phase}"; | ||
|
||
public override IEnumerable<string> Dump() | ||
{ | ||
var lines = new List<string> | ||
{ | ||
DumpHeadline(), | ||
}; | ||
|
||
lines.AddRange(DumpSummary().Select(line => $"\t{line}")); | ||
|
||
return lines; | ||
} | ||
|
||
public void AssertCounter(ICounterProbe probe) => AssertCounterData?.Invoke(this, probe); | ||
|
||
public void AssertCounter() | ||
{ | ||
if (!IsEnabled) | ||
{ | ||
return; | ||
} | ||
|
||
if (!_postponedCounterExceptions.IsEmpty) | ||
{ | ||
throw new AggregateException( | ||
"There were exceptions out of the test execution context.", | ||
_postponedCounterExceptions); | ||
} | ||
|
||
AssertCounter(this); | ||
} | ||
|
||
public void PostponeCounterException(Exception exception) => _postponedCounterExceptions.Add(exception); | ||
|
||
private void ProbeCaptureCompleted(ICounterProbe probe) => | ||
probe.Dump().ForEach(_testOutputHelper.WriteLine); | ||
} |
34 changes: 34 additions & 0 deletions
34
Lombiq.Tests.UI/Services/Counters/Configuration/CounterConfiguration.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
using System; | ||
|
||
namespace Lombiq.Tests.UI.Services.Counters.Configuration; | ||
|
||
public class CounterConfiguration | ||
{ | ||
/// <summary> | ||
/// Gets or sets the counter assertion method. | ||
/// </summary> | ||
public Action<ICounterDataCollector, ICounterProbe> AssertCounterData { get; set; } | ||
|
||
/// <summary> | ||
/// Gets or sets the exclude filter. Can be used to exclude counted values before assertion. | ||
/// </summary> | ||
public Func<ICounterKey, bool> ExcludeFilter { get; set; } | ||
|
||
/// <summary> | ||
/// Gets or sets threshold configuration used under navigation requests. See: | ||
/// <see cref="UI.Extensions.NavigationUITestContextExtensions.GoToAbsoluteUrlAsync(UITestContext, Uri, bool)"/>. | ||
/// See: <see cref="NavigationProbe"/>. | ||
/// </summary> | ||
public CounterThresholdConfiguration NavigationThreshold { get; set; } = new(); | ||
|
||
/// <summary> | ||
/// Gets or sets threshold configuration used per <see cref="YesSql.ISession"/> lifetime. See: | ||
/// <see cref="SessionProbe"/>. | ||
/// </summary> | ||
public CounterThresholdConfiguration SessionThreshold { get; set; } = new(); | ||
|
||
/// <summary> | ||
/// Gets or sets threshold configuration used per page load. See: <see cref="RequestProbe"/>. | ||
/// </summary> | ||
public CounterThresholdConfiguration PageLoadThreshold { get; set; } = new(); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generate exception messages that are easier to understand than:
I as an ordinary developer who didn't set up these counter thresholds but just wanted to change something unrelated but accidentally implemented a
SELECT
N+1 issue, should be able to understand what I did wrong.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While the current message is better, it's still cryptic when you first see it.
Start the exception with a message that explains, in plain English, what happened.