Skip to content

[API tests] End tenant sessions before dropping clean test tenant databases - #12777

Open
Prangshuman Das (t-prda) wants to merge 2 commits into
features/646383-stack-d8-apiv1-reliabilityfrom
features/646383-stack-d9-tenant-refresh
Open

Prangshuman Das (t-prda) wants to merge 2 commits into
features/646383-stack-d8-apiv1-reliabilityfrom
features/646383-stack-d9-tenant-refresh

Conversation

@t-prda

@t-prda Prangshuman Das (t-prda) commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Review focus: Reset-BcTestTenant in build/scripts/ParallelTestExecution.psm1 (+ Pester tests).

Area fix: deterministic clean-tenant refresh. Before each RequiredTestIsolation=Disabled codeunit, clean-tenant execution drops and re-copies the worker tenant database. The container's Remove-NavDatabase runs ALTER DATABASE ... SET OFFLINE WITH ROLLBACK IMMEDIATE with no query timeout. In stack runs it intermittently blocked for 1,763-1,785 s on tenant2 (the server's 30-minute SQL command timeout), e.g. 4x in one IS IntegrationTests job on #12684, adding about 2 hours to that job.

Change:

  • End the tenant's server sessions (Remove-NAVServerSession) before Dismount-NAVTenant.
  • Replace Remove-NAVDatabase with one bounded query (120 s timeout, existing 3 retries): kill the remaining user SQL sessions on the database (by context and by held locks), SET SINGLE_USER WITH ROLLBACK IMMEDIATE, DROP DATABASE. It does nothing when the database does not exist.
  • Log each ended session (program, status, wait) and the drop time, so any remaining stall can be diagnosed.

Validation: Pester 89/89. The T-SQL ran against SQL LocalDB with one connection holding an open transaction in the database and another idle in it: both sessions were ended and the database dropped in 0.17 s. Rerunning on a missing database is a no-op.

Related: AB#646383

Stack (each PR shows only its own layer; merge bottom first)

  1. [API tests] Default to web service key authentication #12674 A: web service key auth (base: main)
  2. [Expense tests] Re-enable Expense Agent API tests #12675 B: Expense API tests + clean test codeunit execution
  3. [API tests] Re-enable APIV1 and APIV2 API tests #12676 C: re-enable APIV1/APIV2 API tests
  4. [API tests] Simplify API test web service key handling #12890 Review fixes for A: simplify web service key handling
  5. Area fixes: [API tests] Ensure VAT posting setup in API document fixtures #12677 D1 VAT fixtures → [API tests] Ensure cancellation reason codes in API sales document tests #12678 D2 cancellation reasons → [API tests] Handle localized purchase invoice payment discounts in APIV1 tests #12679 D3 localized discounts → [API tests] Set sales quote document and posting dates in API tests #12680 D4 quote dates → [API tests] Assign a number series to API vendor payment journals #12681 D5 journal numbering → [API tests] Ignore localized fields in API document comparisons #12682 D6 field comparison → [API tests] Return shipment PDF selection and pdfDocument test isolation #12683 D7 PDF documents → [API tests] Improve APIV1 fixture and verification reliability #12684 D8 APIV1 reliability → this PR, D9 tenant refresh

@t-prda
Prangshuman Das (t-prda) requested review from a team October 13, 2026 20:44
@github-actions github-actions Bot added Build: Automation Workflows and other setup in .github folder Build: scripts & configs Build scripts and configuration files AL: Apps (W1) Add-on apps for W1 labels Oct 13, 2026
@github-actions github-actions Bot added this to the Version 30.0 milestone Oct 13, 2026
@t-prda
Prangshuman Das (t-prda) changed the base branch from main to features/646383-stack-d8-apiv1-reliability October 13, 2026 21:02
@t-prda
Prangshuman Das (t-prda) added this pull request to stack #12697 October 13, 2026 21:02
@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🟠\ High\ Severity\ —\ Security}$

The web service key is held in a Text variable while it is retrieved and checked, making the credential visible to debugger inspection and other plain-text handling. Keep the value as SecretText from the Identity Management retrieval through the authentication consumer; the [NonDebuggable] attribute does not replace the required SecretText credential type.

Knowledge:

Posting this finding as an issue comment because inline comment placement failed.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.50.6

@github-actions

Copy link
Copy Markdown
Contributor

$\textbf{🔴\ Critical\ Severity\ —\ Upgrade}$

The enum value with ordinal 1 was removed without an ObsoleteState = Pending -> Removed transition. Existing persisted rows using ordinal 1 can no longer resolve to the same enum member after the extension upgrade. Preserve the ordinal and obsolete the value with an ObsoleteReason and ObsoleteTag instead of deleting it.

Knowledge:

Posting this finding as an issue comment because inline comment placement failed.

👍 useful · ❤️ especially valuable · 👎 wrong - reply with why · AL review agent v1.50.6

@github-actions github-actions Bot removed Build: Automation Workflows and other setup in .github folder AL: Apps (W1) Add-on apps for W1 labels Oct 14, 2026
@github-actions github-actions Bot added Ownership: Needs Review Ownership is Other, low confidence, or needs manual correction Team: Other GitHub request for other area than SCM, Finance or Integration labels Oct 14, 2026
@t-prda
Prangshuman Das (t-prda) force-pushed the features/646383-stack-d9-tenant-refresh branch from 24c7b43 to 8185669 Compare October 14, 2026 09:01
@t-prda
Prangshuman Das (t-prda) force-pushed the features/646383-stack-d9-tenant-refresh branch from 8185669 to e216440 Compare October 14, 2026 09:28
Comment thread build/scripts/tests/ParallelTestExecution.Test.ps1 Fixed
Comment thread build/scripts/tests/ParallelTestExecution.Test.ps1 Fixed
Comment thread build/scripts/tests/ParallelTestExecution.Test.ps1 Fixed
Comment thread build/scripts/tests/ParallelTestExecution.Test.ps1 Fixed
Comment thread build/scripts/tests/ParallelTestExecution.Test.ps1 Fixed
Comment thread build/scripts/tests/ParallelTestExecution.Test.ps1 Fixed
@t-prda Prangshuman Das (t-prda) added the Team: Finance GitHub request for Finance area label Oct 14, 2026
@github-actions github-actions Bot removed the Team: Finance GitHub request for Finance area label Oct 14, 2026
Comment thread build/scripts/ParallelTestExecution.psm1
@t-prda
Prangshuman Das (t-prda) force-pushed the features/646383-stack-d9-tenant-refresh branch from e216440 to 9bc8418 Compare October 14, 2026 09:48
@t-prda
Prangshuman Das (t-prda) force-pushed the features/646383-stack-d9-tenant-refresh branch from 9bc8418 to 2c0c104 Compare October 14, 2026 10:30
@t-prda
Prangshuman Das (t-prda) force-pushed the features/646383-stack-d9-tenant-refresh branch from 2c0c104 to 3557bc7 Compare October 14, 2026 10:49
@t-prda
Prangshuman Das (t-prda) force-pushed the features/646383-stack-d9-tenant-refresh branch from 3557bc7 to aab8e4a Compare October 14, 2026 14:05
@t-prda
Prangshuman Das (t-prda) removed this pull request from stack #12697 October 15, 2026 08:59
@t-prda
Prangshuman Das (t-prda) added this pull request to stack #12891 October 15, 2026 09:00
@t-prda
Prangshuman Das (t-prda) force-pushed the features/646383-stack-d9-tenant-refresh branch from aab8e4a to 2d40b1d Compare October 15, 2026 09:02
Reset-BcTestTenant removed the tenant database with the container's
Remove-NavDatabase, which takes the database offline with ROLLBACK IMMEDIATE
and no query timeout. Intermittently a leftover session kept tenant2 in use
and the removal blocked for about 30 minutes (the server's SQL command
timeout), stretching a test job by up to two hours.

End the tenant's server sessions before dismounting, then drop the database
with one bounded query (120 seconds, retried) that kills the remaining user
sessions on it, switches to SINGLE_USER and drops it. Log the ended sessions
and the drop time so any remaining stall is diagnosable.

AB#646383

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3952f078-a881-4da8-ad96-13b727e48a91
Get-NAVServerSession also returns internal sessions with negative IDs, which
Remove-NAVServerSession rejects. The first one aborted the loop, so tenant
sessions were not ended on most refreshes in CI. Only end sessions with a
non-negative ID and end each one independently.

AB#646383

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 3952f078-a881-4da8-ad96-13b727e48a91
@t-prda
Prangshuman Das (t-prda) force-pushed the features/646383-stack-d9-tenant-refresh branch from 2d40b1d to 7048602 Compare October 15, 2026 12:56

This branch was successfully deployed

1 active (outdated) deployment
triage — e2164409 Deployed Oct 14, 2026 by t-prda via Classify team ownership #7804
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Build: scripts & configs Build scripts and configuration files Ownership: Needs Review Ownership is Other, low confidence, or needs manual correction Team: Other GitHub request for other area than SCM, Finance or Integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants