OSOE-353: Add duplicate SQL query detector in Lombiq.UITestingToolbox #216

dministro · 2022-10-24T06:09:34Z

OSOE-353
Fixes #93

Summary by CodeRabbit

New Features
- Introduced a "Duplicated SQL query detector" to improve test efficiency and resource usage.
- Added configuration settings for UI Testing Toolbox to enhance SQL query monitoring.
- Implemented a new set of tests for detecting duplicated SQL queries.
Enhancements
- Improved test environment with additional configuration options for database command execution thresholds.
- Enhanced navigation and page load testing with new counter data collection and assertion capabilities.
- Expanded testing toolbox with classes and interfaces to support performance metric collection.
Documentation
- Updated the Readme to reflect new testing features and configurations.
Bug Fixes
- Fixed issues related to database command execution tracking during tests.
Refactor
- Refactored database connection and command wrappers to support performance data collection.
Tests
- Added tests for new functionality and performance monitoring features.
Chores
- Updated internal test infrastructure to incorporate new counter data collector services.

…to issue/OSOE-353

Lombiq.Tests.UI/Exceptions/CounterThresholdException.cs

…to issue/OSOE-353

Fixing typos

github-actions · 2022-10-25T15:45:53Z

This pull request has merge conflicts. Please resolve those before requesting a review.

…to issue/OSOE-353

Piedone

Impressive!

Lombiq.Tests.UI/Services/PhaseCounterConfiguration.cs

Piedone · 2022-10-25T19:40:10Z

Lombiq.Tests.UI/Services/PhaseCounterConfiguration.cs

+
+namespace Lombiq.Tests.UI.Services;
+
+public class PhaseCounterConfiguration


Mention this feature in the root Readme.

This still needs to be done.

Piedone · 2022-10-25T19:41:17Z

Lombiq.Tests.UI/Services/UITestContext.cs

+    /// <summary>
+    /// Gets the currently running <see cref="CounterDataCollector"/> instance.
+    /// </summary>
+    public CounterDataCollector CounterDataCollector { get; init; }


This needs to be possible to be completely disabled too.

This still needs to be done. The page change dumps should also only be written when duplicate SQL query detection is enabled.

Lombiq.Tests.UI/Services/CounterDataCollector.cs

Lombiq.Tests.UI/Services/Counters/CounterProbeBase.cs

Piedone · 2022-10-25T20:04:38Z

Lombiq.Tests.UI/Exceptions/CounterThresholdException.cs

+    {
+    }
+
+    private static string FormatMessage(


Generate exception messages that are easier to understand than:

DbExecuteCounterKey
SELECT [Document].* FROM [Document] WHERE [Document].[Type] = @type LIMIT 1
[0]Type = OrchardCore.Settings.SiteSettings, OrchardCore.Settings

IntegerCounterValue value: 26
Counter value is greater then DbCommandExecutionRepetitionThreshold, threshold: 22

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.

While the current message is better, it's still cryptic when you first see it.

SessionProbe, [GET]https://localhost:9341/Admin
Database reader read counter
Query: SELECT [Document].* FROM [Document] WHERE [Document].[Type] = @type LIMIT 1
Parameters: [0]Type = OrchardCore.AdminMenu.Models.AdminMenuList, OrchardCore.AdminMenu
Count: 2
Counter value is greater then SessionThreshold.DbReaderReadThreshold, threshold: 0.

Start the exception with a message that explains, in plain English, what happened.

Lombiq.Tests.UI/Services/CounterConfiguration.cs

Lombiq.Tests.UI/Services/OrchardCoreHosting/OrchardApplicationFactory.cs

Lombiq.Tests.UI/Services/Counters/Data/ProbedDbConnection.cs

Lombiq.Tests.UI/Services/Counters/CounterProbe.cs

…to issue/OSOE-353

Co-authored-by: Zoltán Lehóczky <[email protected]>

…-Toolbox into issue/OSOE-353

…to issue/OSOE-353

Addressing "This needs to be possible to be completely disabled too." Addressing ""human-readable" everywhere." Addressing counting on page load not just navigation request Addressing counting command text with and without parameters Addressing "Keep the order of private fields and ctor assignments the same as the ctor parameters." Addressing "Comment why this is necessary." Addressing "After a page change, write all counters' values to the TestOutput for debugging."

…to issue/OSOE-353

coderabbitai · 2023-12-29T23:31:41Z

@Piedone, no worries at all! I'm here to assist you. If you have any specific areas or files in the pull request that you would like me to review, please let me know. Otherwise, I will proceed with a general review of the changes.

Action performed

Review triggered.

Piedone · 2023-12-29T23:54:12Z

These comments seem useful, please check them out. You can also chat with the bot, see the tips under #216 (comment).

…to issue/OSOE-353

coderabbitai · 2024-01-30T20:07:33Z

Lombiq.Tests.UI/Services/Counters/Configuration/CounterThresholdConfiguration.cs

+    /// <summary>
+    /// Gets or sets a value indicating whether the current threshold configuration and checking is disabled.
+    /// </summary>
+    public bool Disable { get; set; } = true;


The property Disable should be renamed to IsEnabled and its logic inverted for consistency with common API naming conventions. This change also addresses a previous review comment.

- public bool Disable { get; set; } = true; + public bool IsEnabled { get; set; } = false;

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

/// <summary>

/// Gets or sets a value indicating whether the current threshold configuration and checking is disabled.

/// </summary>

public bool Disable { get; set; } = true;

/// <summary>

/// Gets or sets a value indicating whether the current threshold configuration and checking is disabled.

/// </summary>

public bool IsEnabled { get; set; } = false;

coderabbitai · 2024-01-30T20:07:33Z

Lombiq.Tests.UI/Services/Counters/Configuration/CounterConfiguration.cs

+            }),
+    };
+
+    public static bool DefaultExcludeFilter(ICounterKey key) => DefaultExcludeList.Contains(key);


Consider optimizing the DefaultExcludeFilter method if the default exclude list grows large. Using a more efficient lookup method, such as a hash set, could improve performance.

- public static bool DefaultExcludeFilter(ICounterKey key) => DefaultExcludeList.Contains(key); + public static bool DefaultExcludeFilter(ICounterKey key) => DefaultExcludeListHashSet.Contains(key); + private static HashSet<ICounterKey> DefaultExcludeListHashSet = new HashSet<ICounterKey>(DefaultExcludeList);

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change

public static bool DefaultExcludeFilter(ICounterKey key) => DefaultExcludeList.Contains(key);

public static bool DefaultExcludeFilter(ICounterKey key) => DefaultExcludeListHashSet.Contains(key);

private static HashSet<ICounterKey> DefaultExcludeListHashSet = new HashSet<ICounterKey>(DefaultExcludeList);

…to issue/OSOE-353

Piedone · 2024-04-19T17:07:37Z

Lombiq.Tests.UI/Services/Counters/Configuration/CounterConfiguration.cs

+    public CounterThresholdConfiguration NavigationThreshold { get; set; } = new CounterThresholdConfiguration
+    {
+        DbCommandIncludingParametersExecutionCountThreshold = 11,
+        DbCommandExcludingParametersExecutionThreshold = 22,
+        DbReaderReadThreshold = 11,
+    };
+
+    /// <summary>
+    /// Gets or sets threshold configuration used per <see cref="YesSql.ISession"/> lifetime. See:
+    /// <see cref="SessionProbe"/>.
+    /// </summary>
+    public CounterThresholdConfiguration SessionThreshold { get; set; } = new CounterThresholdConfiguration
+    {
+        DbCommandIncludingParametersExecutionCountThreshold = 22,
+        DbCommandExcludingParametersExecutionThreshold = 44,
+        DbReaderReadThreshold = 11,
+    };
+
+    /// <summary>
+    /// Gets or sets threshold configuration used per page load. See: <see cref="PageLoadProbe"/>.
+    /// </summary>
+    public CounterThresholdConfiguration PageLoadThreshold { get; set; } = new CounterThresholdConfiguration
+    {
+        DbCommandIncludingParametersExecutionCountThreshold = 22,
+        DbCommandExcludingParametersExecutionThreshold = 44,
+        DbReaderReadThreshold = 11,
+    };


Please add a piece of docs contrasting these three configs: when would you want to use each?

… instead of KeyValuePair<> Setting default configuration more generic Moving OSOCE default configuration to OrchardCoreUITestExecutorConfiguration

Fine-tuning thresholds in tests

…re exactly matching with the counter values captured during navigation.

…to issue/OSOE-353

…re` -> `RequestProbeMiddleware`

…to issue/OSOE-353

Adding more sample tests

…to issue/OSOE-353

dministro added 2 commits October 23, 2022 20:33

Adding counter infrastructure

f224aff

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

d87e325

…to issue/OSOE-353

0liver reviewed Oct 24, 2022

View reviewed changes

Lombiq.Tests.UI/Exceptions/CounterThresholdException.cs Outdated Show resolved Hide resolved

dministro added 5 commits October 25, 2022 01:06

Using interfaces

34b538b

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

7574e9a

…to issue/OSOE-353

Implementing required exception constructors

beef486

Adjusting tests

6c3602f

Fixing typos

Fixing typo

06f8eaa

github-actions bot added the Merge Conflict label Oct 25, 2022

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

3cd03e3

…to issue/OSOE-353

github-actions bot removed the Merge Conflict label Oct 25, 2022

Piedone requested changes Oct 25, 2022

View reviewed changes

Piedone mentioned this pull request Oct 25, 2022

OSOE-353: Add duplicate SQL query detector in Lombiq.UITestingToolbox Lombiq/Open-Source-Orchard-Core-Extensions#247

Open

dministro and others added 16 commits November 7, 2022 17:31

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

8335d59

…to issue/OSOE-353

Update Lombiq.Tests.UI/Services/CounterDataCollector.cs

914c291

Co-authored-by: Zoltán Lehóczky <[email protected]>

Update Lombiq.Tests.UI/Services/Counters/CounterProbeBase.cs

1100237

Co-authored-by: Zoltán Lehóczky <[email protected]>

Update Lombiq.Tests.UI/Services/Counters/ICounterValue.cs

df3c3c2

Co-authored-by: Zoltán Lehóczky <[email protected]>

Update Lombiq.Tests.UI/Services/Counters/NavigationProbe.cs

1a0fd1a

Co-authored-by: Zoltán Lehóczky <[email protected]>

Update Lombiq.Tests.UI.Samples/Tests/BasicOrchardFeaturesTests.cs

8e61c22

Co-authored-by: Zoltán Lehóczky <[email protected]>

Update Lombiq.Tests.UI/Services/CounterConfiguration.cs

a20311c

Co-authored-by: Zoltán Lehóczky <[email protected]>

Merge branch 'issue/OSOE-353' of https://github.com/Lombiq/UI-Testing…

f033443

…-Toolbox into issue/OSOE-353

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

cac8255

…to issue/OSOE-353

Adding SessionProbe

169fa26

Adding better exclusion and defaults

cc941b0

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

dff34c3

…to issue/OSOE-353

Fixes after merge

33fd4fe

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

f0d8fc0

…to issue/OSOE-353

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

0a8189c

…to issue/OSOE-353

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

c953c8c

…to issue/OSOE-353

coderabbitai bot reviewed Jan 30, 2024

View reviewed changes

Piedone mentioned this pull request Mar 3, 2024

CodeRabbit for AI code reviews OrchardCMS/OrchardCore#15439

Closed

3 tasks

dministro added 4 commits April 9, 2024 18:52

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

3343bd8

…to issue/OSOE-353

Fixes after merge

4f334fe

Fixing analyzer violations

027486d

Fixing analyzer violations again

fa3a95c

Piedone reviewed Apr 19, 2024

View reviewed changes

dministro added 20 commits May 7, 2024 00:00

Adding custom type for storing db command parameters in configuration…

0e4f177

… instead of KeyValuePair<> Setting default configuration more generic Moving OSOCE default configuration to OrchardCoreUITestExecutorConfiguration

Allowing to enable/disable counter subsystem by configuration

849553f

Fine-tuning thresholds in tests

Adding a bit more information to CounterThresholdException message

9203e36

Removing unnecessary Should.NotThrowAsync()

a52cff1

Adding test to demonstrate the scenario when the counter thresholds a…

30b4997

…re exactly matching with the counter values captured during navigation.

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

2d5abae

…to issue/OSOE-353

Removing unnecessary configuration

1b500d8

Renaming PageLoadProbe -> RequestProbe and `PageLoadProbeMiddlewa…

bca3add

…re` -> `RequestProbeMiddleware`

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

a1531d5

…to issue/OSOE-353

Adding more comments

71663b9

Adding more sample tests

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

1dea988

…to issue/OSOE-353

Spelling

2c7e741

Fixing analyzer warnings

a6f685c

Fix wording

3841c67

Merge branch 'dev' of https://github.com/Lombiq/UI-Testing-Toolbox in…

8b4f5e7

…to issue/OSOE-353

Updating CompatibilitySuppressions.xml

61232cd

Fixing analyzer violations

5d7757d

Implementing IDisposable properly

57c05f7

Comment

c6436ac

Fixing analyzer violation

b079d26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OSOE-353: Add duplicate SQL query detector in Lombiq.UITestingToolbox #216

OSOE-353: Add duplicate SQL query detector in Lombiq.UITestingToolbox #216

dministro commented Oct 24, 2022 •

edited by coderabbitai bot

Loading

github-actions bot commented Oct 25, 2022

Piedone left a comment

Piedone Oct 25, 2022

Piedone Oct 24, 2023

Piedone Oct 25, 2022

Piedone Oct 24, 2023

Piedone Oct 25, 2022

Piedone Oct 25, 2023

coderabbitai bot commented Dec 29, 2023

Piedone commented Dec 29, 2023

coderabbitai bot Jan 30, 2024

coderabbitai bot Jan 30, 2024

Piedone Apr 19, 2024


		namespace Lombiq.Tests.UI.Services;

		public class PhaseCounterConfiguration

	public static bool DefaultExcludeFilter(ICounterKey key) => DefaultExcludeList.Contains(key);
	public static bool DefaultExcludeFilter(ICounterKey key) => DefaultExcludeListHashSet.Contains(key);
	private static HashSet<ICounterKey> DefaultExcludeListHashSet = new HashSet<ICounterKey>(DefaultExcludeList);

OSOE-353: Add duplicate SQL query detector in Lombiq.UITestingToolbox #216

Are you sure you want to change the base?

OSOE-353: Add duplicate SQL query detector in Lombiq.UITestingToolbox #216

Conversation

dministro commented Oct 24, 2022 • edited by coderabbitai bot Loading

Summary by CodeRabbit

github-actions bot commented Oct 25, 2022

Piedone left a comment

Choose a reason for hiding this comment

Piedone Oct 25, 2022

Choose a reason for hiding this comment

Piedone Oct 24, 2023

Choose a reason for hiding this comment

Piedone Oct 25, 2022

Choose a reason for hiding this comment

Piedone Oct 24, 2023

Choose a reason for hiding this comment

Piedone Oct 25, 2022

Choose a reason for hiding this comment

Piedone Oct 25, 2023

Choose a reason for hiding this comment

coderabbitai bot commented Dec 29, 2023

Piedone commented Dec 29, 2023

coderabbitai bot Jan 30, 2024

Choose a reason for hiding this comment

coderabbitai bot Jan 30, 2024

Choose a reason for hiding this comment

Piedone Apr 19, 2024

Choose a reason for hiding this comment

dministro commented Oct 24, 2022 •

edited by coderabbitai bot

Loading