Optimize Playwright functional tests with shared multi-tenant server by Skrypt · Pull Request #19081 · OrchardCMS/OrchardCore

Skrypt · 2026-03-28T18:35:47Z

Replace 6 separate CMS server instances (one per recipe fixture) with a single shared server using SaaS multi-tenancy. Each recipe (Blog, Agency, etc.) is now a child tenant with its own URL prefix, eliminating redundant server startups, database creations, and browser-based setup.

Key changes:

Add static CmsServer that starts one CMS host with AutoSetup and creates all recipe tenants upfront during initialization
Configure OrchardCore.AutoSetup via builder.Configuration so the Default tenant is provisioned on first request without browser form filling
Convert EnsureCleanDatabase to async (EnsureCleanDatabaseAsync) and extract it from ConfigureServices into the async StartCmsAsync call path
Update all CMS test classes to navigate via tenant prefix (/{prefix}/)
Refactor SaasFixture to reuse the shared server for its parent tenant

Verified on all databases running locally:

Database	Tests	Result	Duration
SQLite	12/12	Passed	29s
PostgreSQL	12/12	Passed	31s
MySQL	12/12	Passed	42s
SQL Server	12/12	Passed	31s

PR execution time for all databases: 4m 26s

Main branch execution time for all databases: 4m 8s

Comments

To be honest I'm pushing this as a draft because I want to see if we can optimize this to be faster but I don't think it will be making a huge difference. I need to compare with what I merged already.

So actually these changes were supposed to be an optimization for perf and they end up taking more time than what I originally did.

Replace 6 separate CMS server instances (one per recipe fixture) with a single shared server using SaaS multi-tenancy. Each recipe (Blog, Agency, etc.) is now a child tenant with its own URL prefix, eliminating redundant server startups, database creations, and browser-based setup. Key changes: - Add static CmsServer that starts one CMS host with AutoSetup and creates all recipe tenants upfront during initialization - Configure OrchardCore.AutoSetup via builder.Configuration so the Default tenant is provisioned on first request without browser form filling - Convert EnsureCleanDatabase to async (EnsureCleanDatabaseAsync) and extract it from ConfigureServices into the async StartCmsAsync call path - Update all CMS test classes to navigate via tenant prefix (/{prefix}/) - Refactor SaasFixture to reuse the shared server for its parent tenant Verified on all databases: | Database | Tests | Result | Duration | |----------|-------|--------|----------| | SQLite | 12/12 | **Passed** | 29s | | PostgreSQL | 12/12 | **Passed** | 31s | | MySQL | 12/12 | **Passed** | 42s | | SQL Server | 12/12 | **Passed** | 31s |

Copilot

Pull request overview

This PR refactors the Playwright functional CMS recipe tests to reuse a single shared CMS host by enabling SaaS multi-tenancy, creating each recipe as a child tenant accessed via a URL prefix to reduce redundant server/database setup work.

Changes:

Introduces a static shared CmsServer that starts one CMS host (AutoSetup SaaS) and provisions recipe tenants up front.
Updates recipe test fixtures/tests to navigate and authenticate via /{tenantPrefix}.
Moves external DB cleanup into an async pre-host-build path and wires AutoSetup configuration through builder.Configuration.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
test/OrchardCore.Tests.Functional/Tests/Cms/SaasFixture.cs	Switches SaaS tests to use the shared `CmsServer` lifecycle and browser/server.
test/OrchardCore.Tests.Functional/Tests/Cms/MigrationsTests.cs	Uses the recipe-tenant URL prefix for navigation and admin auth.
test/OrchardCore.Tests.Functional/Tests/Cms/HeadlessTests.cs	Uses the recipe-tenant URL prefix for navigation and admin auth.
test/OrchardCore.Tests.Functional/Tests/Cms/ComingSoonTests.cs	Uses the recipe-tenant URL prefix for navigation and admin auth.
test/OrchardCore.Tests.Functional/Tests/Cms/CmsServerFixture.cs	Adds the shared static CMS server initializer + tenant provisioning logic.
test/OrchardCore.Tests.Functional/Tests/Cms/CmsRecipeFixture.cs	Refactors recipe fixtures to acquire/release the shared server and expose `Prefix`.
test/OrchardCore.Tests.Functional/Tests/Cms/BlogTests.cs	Uses the recipe-tenant URL prefix for navigation and admin auth.
test/OrchardCore.Tests.Functional/Tests/Cms/AgencyTests.cs	Uses the recipe-tenant URL prefix for navigation and admin auth.
test/OrchardCore.Tests.Functional/Helpers/OrchardTestServer.cs	Adds AutoSetup configuration support and async DB cleanup prior to host build.
test/OrchardCore.Tests.Functional/Helpers/OrchardTestFixture.cs	Passes the new AutoSetup recipe option into `StartCmsAsync`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-28T18:56:27Z

test/OrchardCore.Tests.Functional/Tests/Cms/CmsServerFixture.cs

+    public static IBrowser Browser => _testFixture?.Browser;
+    public static string BaseUrl => _testFixture?.BaseUrl;


Browser and BaseUrl are declared non-nullable but can currently return null when AcquireAsync() hasn’t run (or after ReleaseAsync()), which can lead to confusing NREs in consumers. Consider throwing a clear InvalidOperationException when _testFixture is null (or make these properties nullable and update call sites accordingly).

Copilot · 2026-03-28T18:56:27Z

test/OrchardCore.Tests.Functional/Tests/Cms/CmsServerFixture.cs

+            _refCount++;
+            if (_testFixture is not null)
+            {
+                if (_initError is not null)
+                {
+                    throw new InvalidOperationException("CmsServer initialization failed previously.", _initError);
+                }


AcquireAsync() increments _refCount before checking whether initialization previously failed. If _initError is set, the method throws but the ref-count stays incremented, which can prevent the server from ever being disposed. Suggest checking _initError before incrementing, or decrementing _refCount on the error path.

Copilot · 2026-03-28T18:56:27Z

test/OrchardCore.Tests.Functional/Tests/Cms/CmsServerFixture.cs

+            }
+            catch (Exception ex)
+            {
+                _testFixture = fixture; // Mark as attempted so we don't retry.


When fixture.InitializeAsync() throws, the catch block stores _testFixture/_initError but never disposes the partially created OrchardTestFixture, leaking Playwright/browser/server resources. It’d be safer to await fixture.DisposeAsync() in the catch (and also reset/clear _initError when a subsequent successful initialization occurs).

Suggested change

}

catch (Exception ex)

{

_testFixture = fixture; // Mark as attempted so we don't retry.

// Clear any previous initialization error on successful initialization.

_initError = null;

}

catch (Exception ex)

{

try

{

await fixture.DisposeAsync();

}

catch

{

// Ignore disposal exceptions to preserve the original initialization error.

}

Copilot · 2026-03-28T18:56:28Z

test/OrchardCore.Tests.Functional/Tests/Cms/CmsServerFixture.cs

+            _refCount--;
+            if (_refCount <= 0 && _testFixture is not null)
+            {
+                await _testFixture.DisposeAsync();
+                _testFixture = null;
+                _tenantPrefixes = null;


ReleaseAsync() disposes the shared fixture but doesn’t reset _initError/_refCount. If initialization previously failed and the fixture is later released, a subsequent successful initialization would still leave _initError set and cause all future AcquireAsync() calls to throw. Consider clearing _initError when tearing down (and optionally guarding against _refCount going negative).

Suggested change

_refCount--;

if (_refCount <= 0 && _testFixture is not null)

{

await _testFixture.DisposeAsync();

_testFixture = null;

_tenantPrefixes = null;

if (_refCount > 0)

{

_refCount--;

}

else

{

_refCount = 0;

}

if (_refCount == 0 && _testFixture is not null)

{

await _testFixture.DisposeAsync();

_testFixture = null;

_tenantPrefixes = null;

_initError = null;

Copilot · 2026-03-28T18:56:28Z

test/OrchardCore.Tests.Functional/Helpers/OrchardTestServer.cs

+            builder.Configuration["OrchardCore:OrchardCore_AutoSetup:Tenants:0:ShellName"] = "Default";
+            builder.Configuration["OrchardCore:OrchardCore_AutoSetup:Tenants:0:SiteName"] = $"Testing {autoSetupRecipe}";
+            builder.Configuration["OrchardCore:OrchardCore_AutoSetup:Tenants:0:SiteTimeZone"] = "America/New_York";
+            builder.Configuration["OrchardCore:OrchardCore_AutoSetup:Tenants:0:AdminUsername"] = config.Username;
+            builder.Configuration["OrchardCore:OrchardCore_AutoSetup:Tenants:0:AdminEmail"] = config.Email;
+            builder.Configuration["OrchardCore:OrchardCore_AutoSetup:Tenants:0:AdminPassword"] = config.Password;
+            builder.Configuration["OrchardCore:OrchardCore_AutoSetup:Tenants:0:DatabaseProvider"] = dbProvider;
+            builder.Configuration["OrchardCore:OrchardCore_AutoSetup:Tenants:0:DatabaseConnectionString"] = connString;
+            builder.Configuration["OrchardCore:OrchardCore_AutoSetup:Tenants:0:RecipeName"] = autoSetupRecipe;


AutoSetup sets SiteTimeZone to a hard-coded IANA ID ("America/New_York"). This can be brittle across environments (e.g., differing time zone ID support/config) and it isn’t required for these tests to be location-specific. Consider using a more universally supported value like "UTC" (or deriving from TimeZoneInfo.Utc.Id) to reduce setup fragility.

Piedone · 2026-03-28T19:38:02Z

I don't recommend this. UI tests, as unit tests, are best to run from a completely clean state, so they don't interfere with each other, causing false positives or it being unclear which recipe/test a given issue corresponds to. Also, using sub-tenants for every test can mask issues of using them with a single Default tenant, which is the primary use case.

Skrypt requested a review from Copilot March 28, 2026 18:51

Copilot started reviewing on behalf of Skrypt March 28, 2026 18:52 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize Playwright functional tests with shared multi-tenant server#19081

Optimize Playwright functional tests with shared multi-tenant server#19081
Skrypt wants to merge 1 commit intomainfrom
skrypt/playwright-optimize

Skrypt commented Mar 28, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Piedone commented Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		public static IBrowser Browser => _testFixture?.Browser;
		public static string BaseUrl => _testFixture?.BaseUrl;

-            }
-            catch (Exception ex)
-            {
-                _testFixture = fixture; // Mark as attempted so we don't retry.
+                // Clear any previous initialization error on successful initialization.
+                _initError = null;
+            }
+            catch (Exception ex)
+            {
+                try
+                {
+                    await fixture.DisposeAsync();
+                }
+                catch
+                {
+                    // Ignore disposal exceptions to preserve the original initialization error.
+                }

Conversation

Skrypt commented Mar 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Comments

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Piedone commented Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Skrypt commented Mar 28, 2026 •

edited

Loading