Add version-agnostic upgrade/persistence compatibility test suite - #2715
Open
Akanksha Jain (jainakanksha-msft) wants to merge 19 commits into
Open
Add version-agnostic upgrade/persistence compatibility test suite#2715Akanksha Jain (jainakanksha-msft) wants to merge 19 commits into
Akanksha Jain (jainakanksha-msft) wants to merge 19 commits into
Conversation
- npm/VSIX/Docker upgrade scenarios for blob, queue, and table data - shared UpgradeTarget abstraction (NpmProcessTarget/DockerContainerTarget) - shared blobUploader/tableValueCodec fixture handling across npm and Docker - dedicated CI workflow running on merge to main and on demand
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 6, 2026 11:40
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new tests/upgrade/ regression suite and CI workflow to continuously validate Azurite’s persistence/upgrade compatibility across releases by seeding data with the latest published artifacts (npm, Marketplace VSIX, MCR Docker image) and verifying it remains readable and byte/value-identical after upgrading to the local build.
Changes:
- Added version-agnostic upgrade tests for blob/queue/table (npm) and Docker image upgrade (MCR → local image with shared volume).
- Added a VSIX lifecycle test harness using
@vscode/test-electron(install VSIX → activate → start/stop services). - Added a dedicated GitHub Actions workflow plus npm scripts to run the new upgrade suites.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/upgrade/vsixLifecycle/suite/vsixLifecycle.test.js | VSIX lifecycle assertions (activate/start/stop) driven via VS Code Extension Host |
| tests/upgrade/vsixLifecycle/suite/index.js | Mocha test loader for the VSIX lifecycle suite |
| tests/upgrade/vsixLifecycle/runVsixTests.ts | Downloads VS Code test instance, installs VSIX into isolated profile, runs lifecycle suite |
| tests/upgrade/vsixLifecycle/resolveVsixToTest.ts | Resolves/creates the VSIX under test (local package vs Marketplace download) |
| tests/upgrade/vsixLifecycle/driverExtension/package.json | Minimal driver extension manifest used only to host tests |
| tests/upgrade/vsixLifecycle/driverExtension/extension.js | No-op activation entrypoint for the driver extension |
| tests/upgrade/utils/versionResolver.ts | Dynamic resolution of latest published npm/Marketplace/MCR versions |
| tests/upgrade/utils/upgradeTarget.ts | Common start/stop abstraction for npm process vs Docker container targets |
| tests/upgrade/utils/tableValueCodec.ts | Shared typed table entity payload building + verification helpers |
| tests/upgrade/utils/processHarness.ts | Generic Azurite CLI process spawn/readiness/stop harness |
| tests/upgrade/utils/npmVersionInstaller.ts | Installs a specific azurite npm version into an isolated temp directory |
| tests/upgrade/utils/integrity.ts | Byte/hash comparison and property map comparison helpers |
| tests/upgrade/utils/httpProbe.ts | HTTP readiness probing shared by harnesses |
| tests/upgrade/utils/dockerHarness.ts | Thin docker CLI wrapper for pull/build/run/stop/rm |
| tests/upgrade/utils/dataFixtures.ts | Deterministic blob/queue/table fixtures for cross-version verification |
| tests/upgrade/utils/blobUploader.ts | Shared blob seeding + byte-for-byte verification across blob types |
| tests/upgrade/tableUpgrade.test.ts | Npm-based table upgrade compatibility test (old published → local build) |
| tests/upgrade/queueUpgrade.test.ts | Npm-based queue upgrade compatibility test (old published → local build) |
| tests/upgrade/dockerUpgrade.test.ts | Docker/MCR upgrade test (old MCR tag → locally built image on same volume) |
| tests/upgrade/blobUpgrade.test.ts | Npm-based blob upgrade compatibility test (old published → local build) |
| tests/blob/upgradeRegression.test.ts | Updates existing upgrade regression test string to be version-agnostic |
| package.json | Adds @vscode/test-electron dev dependency and upgrade test scripts |
| package-lock.json | Locks new @vscode/test-electron transitive dependencies |
| docs/designs/2026-08-upgrade-compatibility-testing.md | Design doc describing architecture and rationale for the upgrade suite |
| ChangeLog.md | Notes the addition of the new upgrade compatibility regression suite |
| .github/workflows/UpgradeCompatibility.yml | New CI workflow to run upgrade compatibility jobs post-merge and on demand |
Suppressed comments (2)
tests/upgrade/vsixLifecycle/suite/vsixLifecycle.test.js:68
- This test claims it "stops all services" but it only verifies the blob port stops responding. Queue/table could remain running and the test would still pass.
const isDown = await waitUntil(
async () => !(await probeHttp(BLOB_DEFAULT_PORT)),
30000,
1000
);
tests/upgrade/utils/processHarness.ts:91
- AzuriteProcessHandle.stop() awaits the child process 'exit' event with no upper bound. If the process fails to terminate (including after the SIGKILL attempt), the promise never resolves and the whole test run can hang indefinitely.
await new Promise<void>((resolve) => {
child.once("exit", () => resolve());
child.kill();
setTimeout(() => {
if (child.exitCode === null) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- DockerContainerTarget.start() now waits for blob, queue, and table ports instead of only blob, matching the UpgradeTarget contract - AzuriteProcessHandle.start() now rejects on any early exit (including code 0) instead of only non-zero exit codes - VSIX lifecycle test now probes all three default ports on start/stop instead of only the blob port - resolveVsixToTest.ts uses npx.cmd on Windows instead of hardcoding npx
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 6, 2026 12:11
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Suppressed comments (3)
tests/upgrade/utils/tableValueCodec.ts:67
assertEntityMatchesFixture()assumes the SDK returnsbinaryPropas aUint8Array. In practice@azure/data-tablesmay return either raw bytes (Buffer/Uint8Array) or a base64 string depending on serialization shape; if it returns a string,Buffer.from(string)will interpret it as UTF-8 and the comparison will be incorrect.
const fetchedBinary = unwrapTypedValue(fetched.binaryProp);
assert.deepStrictEqual(
Buffer.from(fetchedBinary as Uint8Array),
Buffer.from(entity.binaryProp)
);
tests/upgrade/utils/tableValueCodec.ts:24
toCreateEntityPayload()base64-encodesbinaryPropbefore passing it as an ODataBinarytyped value.@azure/data-tablesalready handlesBinaryencoding when the value is bytes (Buffer/Uint8Array); pre-encoding here risks double-encoding and storing different bytes than the fixture intended.
This issue also appears on line 63 of the same file.
binaryProp: {
value: Buffer.from(entity.binaryProp).toString("base64"),
type: "Binary" as const
}
tests/upgrade/utils/versionResolver.ts:150
- MCR/Docker Registry pagination uses a relative URL in the
Link: <...>; rel="next"header (e.g./v2/<repo>/tags/list?...). Assigning that directly tourlwill make the nextfetch()fail because it isn't an absolute URL.
const link = res.headers.get("link");
const nextMatch = link?.match(/<([^>]+)>;\s*rel="next"/);
url = nextMatch ? nextMatch[1] : undefined;
Will be reverted before merge - only needed to prove the workflow YAML itself runs correctly since workflow_dispatch can't register until this file exists on main.
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 6, 2026 12:17
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Suppressed comments (3)
tests/upgrade/utils/versionResolver.ts:90
getLatestPublishedMarketplaceVersionsorts arbitrary Marketplace version strings usingcompareSemver, but it doesn't filter out non-plain semver values. If the Marketplace ever returns a non-x.y.zversion (preview/suffixed),compareSemvercan produceNaNand the "latest" selection becomes unreliable. Filter to plain semver (consistent with Docker tag logic) before sorting.
const filtered = versions
.filter((v) => v !== excludeVersion)
.sort(compareSemver);
tests/upgrade/utils/tableValueCodec.ts:61
@azure/data-tablesretrieves DateTime properties asDateobjects (see existing table tests), but this code coercesfetched.dateProptostringand reparses it. ParsingDate#toString()output is implementation-dependent and can be flaky across environments. Compare usingDatedirectly when available, and only parse if a string was returned.
assert.strictEqual(
new Date(fetched.dateProp as string).getTime(),
entity.dateProp.getTime()
);
.github/workflows/UpgradeCompatibility.yml:12
- PR description says this workflow should not run on
pull_request, but the workflow currently includes apull_requesttrigger (even if intended to be temporary). This will run the expensive upgrade suite on every PR update, contradicting the stated intent. Remove thepull_requesttrigger before merging.
# TEMPORARY: pull_request added to validate this workflow runs correctly on
# PR #2715 before merge. Remove this trigger before merging.
# Runs after a PR is merged to main (push), and on demand via workflow_dispatch.
on:
push:
branches:
- main
pull_request:
workflow_dispatch:
- DockerImageUpgrade_Ubuntu: docker run replaces the image's CMD entirely (no ENTRYPOINT is set), so no CLI args reached the containerized Azurite process. Re-specify the default startup args plus --skipApiVersionCheck so an older published image doesn't reject the SDK client's x-ms-version. - UpgradeCompatibility_Windows: spawning npm.cmd/npx.cmd directly without shell: true throws EINVAL on Windows (Node.js CVE-2024-27980 hardening). Added shell: true for the win32 case in both spawn sites.
…ames - Give every workflow step a descriptive name instead of the default 'Run <command>' label - processHarness.stop(): add a hard upper-bound timeout so a process that ignores SIGKILL can never hang the test run indefinitely - tableValueCodec: normalize Binary read-back to handle either raw bytes or a base64 string depending on SDK serialization shape; compare dateProp using instanceof Date before falling back to string parsing - versionResolver: resolve MCR pagination Link header URLs against an absolute base (they can be relative), and filter Marketplace versions to plain semver before sorting, consistent with the Docker tag logic
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 6, 2026 12:40
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Suppressed comments (2)
.github/workflows/UpgradeCompatibility.yml:12
- The PR description says this workflow intentionally does not run on
pull_request, but the workflow currently includes apull_request:trigger (marked “TEMPORARY”). This will run the expensive upgrade suite on every PR update unless removed before merge.
# TEMPORARY: pull_request added to validate this workflow runs correctly on
# PR #2715 before merge. Remove this trigger before merging.
# Runs after a PR is merged to main (push), and on demand via workflow_dispatch.
on:
push:
branches:
- main
pull_request:
workflow_dispatch:
tests/upgrade/vsixLifecycle/runVsixTests.ts:61
resolveVsixToTest()can create a temporary directory for a packaged/downloaded VSIX, butrunVsixTests.tsonly cleans up the VS Code user-data / extensions / workspace dirs. This leaves behindazurite-local-vsix-*andazurite-marketplace-vsix-*temp folders on every run.
} finally {
rmSync(userDataDir, { recursive: true, force: true });
rmSync(extensionsDir, { recursive: true, force: true });
rmSync(workspaceDir, { recursive: true, force: true });
}
…ess termination - dockerUpgrade test: reset bind-mounted volume ownership back to the host runner user before removal, since the container writes as root and the CI runner user can't unlink root-owned files (EACCES) - processHarness: use taskkill without /F on Windows to request a graceful shutdown so Azurite can flush/close its persistence layer before exiting; child.kill() on Windows always force-terminates and never lets the SIGTERM handler run
resolveVsixToTest() now returns the temp directory it created (if any) alongside the vsix path, so runVsixTests.ts can remove it in the finally block. Previously azurite-local-vsix-* and azurite-marketplace-vsix-* directories were left behind on every run.
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 6, 2026 13:17
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Suppressed comments (3)
tests/upgrade/utils/processHarness.ts:107
- In
stop(), theonExithandler referencesforceKillTimer/giveUpTimerbefore thoseconstbindings are initialized. If the child exits quickly, this can throw aReferenceErrordue to the temporal dead zone. Initialize timer variables before wiringonExit, or useletwith undefined checks and create the timers before callingrequestGracefulStop().
const onExit = () => {
clearTimeout(forceKillTimer);
clearTimeout(giveUpTimer);
resolve();
};
tests/upgrade/vsixLifecycle/runVsixTests.ts:42
resolveCliArgsFromVSCodeExecutablePath()can return a.cmd/.batCLI on Windows, and Node may fail to spawn it withoutshell: true(same issue you already handled fornpx.cmd). Add a Windows-onlyshelloption here sonpm run test:upgrade:vsixis runnable on Windows too.
{ stdio: "inherit" }
.github/workflows/UpgradeCompatibility.yml:12
- The PR description says this workflow intentionally does not run on
pull_request, but the workflow currently includes apull_requesttrigger (and even notes it as temporary). This will add the expensive runs back onto every PR. Remove the temporary comment and thepull_requesttrigger before merging.
# TEMPORARY: pull_request added to validate this workflow runs correctly on
# PR #2715 before merge. Remove this trigger before merging.
# Runs after a PR is merged to main (push), and on demand via workflow_dispatch.
The earlier taskkill-based approach still didn't reliably trigger graceful shutdown on Windows: taskkill without /f only delivers WM_CLOSE, which console (non-GUI) processes don't handle, so it fails and silently falls back to a hard kill - explaining why Windows CI still lost persisted data on stop. src/azurite.ts already supports a 'shutdown' IPC message for exactly this purpose (used elsewhere, e.g. the VS Code extension), which works identically on every platform since it doesn't depend on OS signal delivery at all. Switch to fork() so the IPC channel exists, and send 'shutdown' to request a clean close; SIGKILL remains the last-resort timeout fallback.
- processHarness.stop(): declare forceKillTimer/giveUpTimer with let before onExit is defined, instead of relying on them being assigned later in the same synchronous block before the exit event can fire. Avoids a ReferenceError if that ordering assumption ever breaks. - runVsixTests: resolveCliArgsFromVSCodeExecutablePath() can resolve to a .cmd/.bat wrapper on Windows; add shell: true there too, same as the existing npx.cmd handling.
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 6, 2026 23:37
View session
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 7, 2026 06:24
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated 1 comment.
Suppressed comments (10)
tests/upgrade/utils/versionResolver.ts:124
- This has the same upgrade-direction problem as the npm resolver: on an older checkout, the selected MCR tag may be newer than the local image, so the scenario validates a downgrade instead. Filter for tags strictly below the local version.
const filtered = tags
.filter((t) => SEMVER_TAG_PATTERN.test(t) && t !== excludeVersion)
.sort(compareSemver);
.github/workflows/UpgradeCompatibility.yml:12
- The temporary
pull_requesttrigger contradicts both the PR description and this file's own instruction to remove it before merge. As written, all four costly jobs run on every PR update rather than only after merges or on demand.
pull_request:
.github/workflows/UpgradeCompatibility.yml:57
- This job runs
test:upgrade:vsix, whose default mode packages the local tree; it never runs the addedpublished-latestmode. Consequently the dedicated workflow does not install or exercise the latest Marketplace VSIX as claimed in the PR description and changelog. Run both modes (or use a matrix) so local packaging and published-package coverage are both retained.
run: xvfb-run -a npm run test:upgrade:vsix
tests/upgrade/dockerUpgrade.test.ts:180
- The Docker path verifies only the queue's approximate count, so corrupted or changed message values would still pass. This conflicts with the stated value-for-value verification and with the npm queue scenario. Receive and compare the seeded messages here as well.
const queueClient = makeQueueClient(QUEUE_PORT, queueFixture.queueName);
const properties = await queueClient.getProperties();
assert.strictEqual(
properties.approximateMessagesCount,
queueFixture.messages.length,
"Queue message count did not survive the docker image upgrade"
);
tests/upgrade/utils/upgradeTarget.ts:87
- If
docker runfails after creating the container, or any readiness probe rejects,start()exits without removing it. The test callsstart()before itstry/finally, so the named container remains and can keep ports and the bind mount in use. Clean up the container on every startup failure.
runContainer(this.options);
await Promise.all([
tests/upgrade/utils/versionResolver.ts:37
- Excluding only the local version does not guarantee an upgrade. When this suite is run from a stale branch or older commit, the highest remaining published version can be newer than the local build, turning the test into a downgrade. Select the newest stable version strictly below the local baseline.
This issue also appears on line 122 of the same file.
const versions = Object.keys(json.versions ?? {})
.filter((v) => v !== excludeVersion && !v.includes("-"))
.sort(compareSemver);
tests/upgrade/vsixLifecycle/resolveVsixToTest.ts:38
published-latestinvokes a resolver that excludes the local package version. Once that same version is published, this mode deliberately downloads the previous Marketplace release rather than the latest one, despite its name and documented behavior. The Marketplace lifecycle resolver needs a true "latest" mode without local-version exclusion; keep any older-than-local selection separate for upgrade scenarios.
const version =
mode === "published-latest"
? await getLatestPublishedMarketplaceVersion()
: mode;
docs/designs/2026-08-upgrade-compatibility-testing.md:29
- This coverage claim is incorrect. The VSIX lifecycle runner creates a fresh workspace and neither receives the npm suite's data location nor seeds/reads persisted fixtures, so it does not verify that old-version data is readable by a VSIX. Implement the cross-surface persistence scenario or mark this requirement as not covered.
| 2 | Data created by an old version is readable by the latest VSIX | `tests/upgrade/vsixLifecycle/` (installs a VSIX pointed at the same on-disk location seeded by step 1) |
tests/upgrade/vsixLifecycle/runVsixTests.ts:29
- In
@vscode/test-electron2.5.2 this helper adds its own.vscode-test--user-data-dirand--extensions-dirunlessreuseMachineInstallis true. The install command then appends a second pair of those flags, whilerunTestsuses only the custom temp directories. Depending on duplicate-argument handling, the VSIX can be installed into a different profile andgetExtensionwill fail. Suppress the helper defaults so installation and execution unambiguously share the same directories.
const [cli, ...cliArgs] =
resolveCliArgsFromVSCodeExecutablePath(vscodeExecutablePath);
tests/upgrade/utils/processHarness.ts:46
- On a readiness timeout this removes listeners and rejects but leaves the forked Azurite process running. All callers await
start()before entering theirtry/finally, so this failure path leaks the process and can keep ports/data files locked, especially on Windows. Terminate the failed child and wait for its exit before rejecting.
const timer = setTimeout(() => {
cleanup();
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Suppressed comments (4)
tests/upgrade/utils/versionResolver.ts:24
- This strict comparison drops the currently published release whenever its version matches
package.json—the normal state onmainhere (both are 3.36.0). Consequently the npm and MCR suites seed 3.35.x data instead of the latest 3.36.0 data promised by the PR, so regressions specific to the newest persistence format can be missed. Treat an equal-version published artifact as an older code generation (select the newest published version<=the local package version), while still rejecting versions greater than a stale checkout.
/** True if `version` sorts strictly before `baseline` per semver ordering. */
function isOlderThan(version: string, baseline: string): boolean {
return compareSemver(version, baseline) < 0;
}
.github/workflows/UpgradeCompatibility.yml:12
- The comment directly above marks this trigger as temporary, and the PR description says this expensive workflow deliberately must not run on every pull request. Leaving
pull_requestenabled makes all four jobs run on each PR update rather than only after merges and on demand.
pull_request:
.github/workflows/UpgradeCompatibility.yml:57
- This job invokes the default local-VSIX mode, so
test:upgrade:vsix:publishedand the Marketplace resolver are never exercised by this workflow. That leaves the PR's stated validation of the latest published Marketplace VSIX absent from both merge-time and workflow-dispatch runs. Add a published-VSIX invocation (or explicitly narrow the documented scope) without dropping the local packaging lifecycle check.
- name: Run VSIX install/activate/start/stop lifecycle test
run: xvfb-run -a npm run test:upgrade:vsix
tests/upgrade/utils/upgradeTarget.ts:95
runContainer()is outside the cleanuptry. Ifdocker runcreates the container but fails to start it (for example, because a port is occupied), Docker can leave that container behind,start()rejects before callers enter theirtry/finally, and the cleanup path is never called. Include container creation in thistryso every startup failure removes the named container.
runContainer(this.options);
try {
- versionResolver.ts: allow selecting a published version equal to the local package version (previously an equal version was excluded, causing release-day/synced-checkout runs to seed with a stale release instead of the intended one). Still rejects any published version newer than local to avoid testing a downgrade on stale checkouts. Applies to both npm and MCR/Docker tag resolution. - upgradeTarget.ts: move runContainer() inside the try block so a failing `docker run` (e.g. port already in use) still triggers cleanup instead of leaking a partially-created container. - UpgradeCompatibility.yml: also exercise the published Marketplace VSIX in the VsixLifecycle job, not just the locally packaged one. - design doc: reflect the equal-version-allowed resolution semantics.
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 7, 2026 07:20
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Suppressed comments (13)
docs/designs/2026-08-upgrade-compatibility-testing.md:122
- This says the selected Docker tag excludes the local version, but
getLatestPublishedDockerTag()explicitly accepts tags equal to the local version (compareSemver(...) <= 0). Update the design description to reflect the implemented upper bound.
`versionResolver.ts` additionally exposes `getLatestPublishedDockerTag()`, which paginates the public MCR
tags API (`https://mcr.microsoft.com/v2/azure-storage/azurite/tags/list`) and picks the newest plain
semver tag (excluding `-amd64`/`-arm64`/`-preview` suffixes, `latest`, and the local version).
docs/designs/2026-08-upgrade-compatibility-testing.md:140
- This core design statement conflicts with the resolver: npm/MCR may select a version equal to local, while Marketplace deliberately returns the true latest without comparing to local. Describe those policies separately rather than saying both select a version different from
package.json.
1. **No hardcoded versions.** `versionResolver.ts` queries the npm registry
(`GET https://registry.npmjs.org/azurite`) and the Marketplace gallery API for the newest version
that isn't the local `package.json` version. This means the suite automatically "just works" on every
future release without code changes - it always tests "latest public release -> whatever is checked
out locally", which is exactly the upgrade path real users experience.
.github/workflows/UpgradeCompatibility.yml:12
- The workflow still runs on every pull request, contrary to the PR's stated merge-only/on-demand policy and the adjacent “Remove this trigger before merging” note. This would incur all four expensive upgrade jobs on each PR update; remove the temporary trigger before merge.
pull_request:
tests/upgrade/utils/blobUploader.ts:75
- This verifier confirms content and content type but never confirms that the persisted blob type survived. A regression that reloads an append/page blob as a block blob would therefore pass despite the suite claiming coverage of all three persistence paths. Assert
properties.blobTypeagainst the fixture type.
const properties = await blobClient.getProperties();
ChangeLog.md:34
- The release note implies the Marketplace VSIX participates in fixture seeding and an in-place upgrade to the local build, but the VSIX suite only performs separate install/activate/start/stop lifecycle runs. Clarify that persistence upgrade validation is npm/Docker-only so the documented coverage matches the implementation and PR description.
- Added a version-agnostic upgrade/persistence compatibility test suite (`tests/upgrade/`, run via `npm run test:upgrade`, `test:upgrade:docker`, `test:upgrade:vsix`) that installs the latest published Azurite (npm, Marketplace VSIX, and Docker/MCR image), seeds blob (block/append/page, txt/json/csv/xml/binary), queue, and table data, upgrades in place to the local build, and verifies byte-for-byte / value-for-value integrity. Added dev dependency `@vscode/test-electron` for the VSIX lifecycle test, and a dedicated `.github/workflows/UpgradeCompatibility.yml` CI workflow that runs on merge to `main` and on demand.
docs/designs/2026-08-upgrade-compatibility-testing.md:107
- These entries claim partial coverage of requirement 2, but the requirements table explicitly says old-data readability through the VSIX is not covered. Remove requirement 2 from these annotations so the directory map does not contradict the stated scope.
blobUpgrade.test.ts # req 1, 2 (partially), 4, 5
queueUpgrade.test.ts # req 1, 2 (partially), 6
tableUpgrade.test.ts # req 1, 2 (partially), 7
docs/designs/2026-08-upgrade-compatibility-testing.md:16
- The summary attributes the local-version cap to npm/Marketplace, but the implementation applies it to npm/MCR and intentionally performs an uncapped Marketplace lookup. Correct this overview so readers get the actual selection policy.
This issue also appears in the following locations of the same file:
- line 120
- line 136
1. Installs the **latest currently-published** version of Azurite that is no newer than the
local build (npm and/or Marketplace VSIX) - so a release-day run where the published
version matches the local version still seeds with that (older code generation) release,
while a stale local checkout never ends up testing a downgrade.
tests/upgrade/blobUpgrade.test.ts:67
- This temp directory is created while Mocha loads the file, before grep filtering. The existing broad
npm testglob loads this file but--grep @lokiskips the suite, so itsafterhook never runs and every normal test invocation leaks this directory. Allocate it inbeforeso it is created only when the upgrade suite actually runs.
const dataLocation = mkdtempSync(join(tmpdir(), "azurite-upgrade-blob-"));
tests/upgrade/tableUpgrade.test.ts:72
- This allocation runs as soon as Mocha loads the module. Normal
npm testincludes the file via its broad glob but filters out@upgrade, so Mocha does not execute this suite's cleanup hook and leaves the temp directory behind. Allocate it insidebeforeinstead.
const dataLocation = mkdtempSync(join(tmpdir(), "azurite-upgrade-table-"));
tests/upgrade/dockerUpgrade.test.ts:115
- The Docker volume directory is allocated during module loading, not when this suite runs. The existing
npm testglob loads this file but--grep @lokifilters out its@upgradetest, so theafterhook is skipped and a directory leaks on every regular test run. Move allocation intobefore.
const volumeHostDir = mkdtempSync(join(tmpdir(), "azurite-upgrade-docker-"));
tests/upgrade/queueUpgrade.test.ts:66
- This temp directory is created during file loading. Because the repository's broad
npm testglob loads this file and then filters out@upgradetests with--grep @loki, Mocha skips the suite hooks and the directory is never removed. Move allocation intobefore.
const dataLocation = mkdtempSync(join(tmpdir(), "azurite-upgrade-queue-"));
tests/upgrade/utils/npmVersionInstaller.ts:21
- If
npm installfails, this directory is never returned,oldInstallDirremains unset in the caller, and the suite's cleanup cannot remove it. Wrap installation intry/catchand recursively removeinstallDirbefore rethrowing so failed network/package installs do not leak temporary trees.
const installDir = mkdtempSync(
join(tmpdir(), `azurite-upgrade-npm-${version}-`)
);
tests/upgrade/vsixLifecycle/resolveVsixToTest.ts:46
- If
vsce packagethrows,outDiris never returned tomain, so itsfinallyblock cannot clean up the partially packaged directory. RemoveoutDirin a localcatchbefore rethrowing to keep failed lifecycle runs hermetic.
const outDir = mkdtempSync(join(tmpdir(), "azurite-local-vsix-"));
const outPath = join(outDir, "azurite-local.vsix");
const npx = process.platform === "win32" ? "npx.cmd" : "npx";
execFileSync(npx, ["vsce", "package", "--out", outPath], {
- blobUpgrade/queueUpgrade/tableUpgrade/dockerUpgrade .test.ts: move mkdtempSync() from describe-body (module load time) into before(). Mocha loads these files under the default `npm test` --grep @loki filter (which excludes @upgrade), but describe-body code still runs during collection while the after() cleanup hook is skipped - leaking a temp dir on every normal test run. Verified fixed by running the files directly under --grep @loki: 0 temp dirs created. - npmVersionInstaller.ts / resolveVsixToTest.ts: wrap the install/ package child-process calls in try/catch and remove the temp dir before rethrowing, so a failed `npm install` or `vsce package` no longer leaks its throwaway directory. - blobUploader.ts: assertBlobFixtureSurvived() now also asserts properties.blobType, so a regression that reloads an append/page blob as a block blob is actually caught. - design doc: fix directory-layout comments claiming partial coverage of requirement 2 (contradicts the requirements table, which says it's not yet covered); clarify that the local-version cap applies to npm/MCR resolution only, not the uncapped Marketplace lookup. - ChangeLog.md: clarify that persistence-upgrade validation is npm/Docker-only; the VSIX suite only covers install/activate/ start/stop lifecycle, not cross-version data persistence.
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 7, 2026 08:08
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 25 out of 26 changed files in this pull request and generated no new comments.
Suppressed comments (5)
.github/workflows/UpgradeCompatibility.yml:12
- Remove the temporary
pull_requesttrigger before merging. The PR description and this workflow's own comment say these expensive jobs should run only after pushes tomainor manual dispatch, but this line currently runs all four jobs on every PR update.
pull_request:
tests/upgrade/blobUpgrade.test.ts:71
- Guard this cleanup when setup never initialized
dataLocation. Mocha runs suite cleanup after a failingbeforehook, so a missing local build ormkdtempSyncfailure currently causesrmSync(undefined, ...)and masks the original setup error.
rmSync(dataLocation, { recursive: true, force: true });
tests/upgrade/queueUpgrade.test.ts:70
- Guard this cleanup when setup never initialized
dataLocation. Mocha runs suite cleanup after a failingbeforehook, so a missing local build ormkdtempSyncfailure currently causesrmSync(undefined, ...)and masks the original setup error.
rmSync(dataLocation, { recursive: true, force: true });
tests/upgrade/tableUpgrade.test.ts:76
- Guard this cleanup when setup never initialized
dataLocation. Mocha runs suite cleanup after a failingbeforehook, so a missing local build ormkdtempSyncfailure currently causesrmSync(undefined, ...)and masks the original setup error.
rmSync(dataLocation, { recursive: true, force: true });
tests/upgrade/dockerUpgrade.test.ts:122
- Handle setup failure before using
volumeHostDirin cleanup. If Docker is unavailable—the explicit error path above—or temp-directory creation fails, Mocha still runs this hook; passing the uninitialized value here masks the useful setup error with a path/argument error.
resetVolumeOwnership(volumeHostDir, LOCAL_IMAGE_TAG);
removeImage(LOCAL_IMAGE_TAG);
rmSync(volumeHostDir, { recursive: true, force: true });
Blob/queue/table upgrade tests each independently installed the same published azurite version via npm, which is especially costly on Windows runners (~2 min per install). Cache the install per version in installNpmVersion() and centralize cleanup in a mocha root hook (rootHooks.ts) instead of each file's own after() hook. Also guard dockerUpgrade's after() against a before() failure leaving volumeHostDir unset, consistent with the other upgrade suites.
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 7, 2026 08:29
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (2)
package.json:334
rootHooks.tsis currently passed as a positional test file, so Mocha does not register its exportedmochaHooksplugin andcleanupCachedNpmInstalls()never runs. Load the hook file with--requireinstead; otherwise each local upgrade run leaves the cached npm installation in the system temp directory.
"test:upgrade": "npm run build && mocha --require ts-node/register --no-timeouts --grep @upgrade --exit tests/upgrade/rootHooks.ts tests/upgrade/blobUpgrade.test.ts tests/upgrade/queueUpgrade.test.ts tests/upgrade/tableUpgrade.test.ts",
.github/workflows/UpgradeCompatibility.yml:12
- The PR description says this expensive workflow must run only after merges and on demand, but this trigger runs all four jobs on every PR update. The adjacent comment also marks it as temporary, so remove it before merging.
pull_request:
Resolves package-lock.json conflict from main's eslint v10 / multistream bumps by regenerating the lockfile against the merged package.json.
Mocha only recognizes an exported mochaHooks plugin when the file is loaded via --require; passing it as a positional spec file (as before) meant cleanupCachedNpmInstalls() never ran, leaking the cached npm install in the OS temp dir on every local run.
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 7, 2026 09:41
View session
# Conflicts: # package-lock.json
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/UpgradeCompatibility.yml:11
- Remove this temporary trigger before merging. The PR description and design explicitly say this expensive suite should run only after pushes to
mainor viaworkflow_dispatch; leavingpull_requestenabled runs all four upgrade jobs on every PR update and contradicts that stated CI policy.
pull_request:
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 7, 2026 09:47
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/designs/2026-08-upgrade-compatibility-testing.md:16
- This summary incorrectly includes the Marketplace VSIX among artifacts capped at the local version and used to seed persisted data. The implementation caps npm/MCR only; Marketplace is intentionally uncapped and is exercised only by the separate lifecycle test, as the requirements table and decision 1 later state. Update this top-level sequence so the design does not describe a different test flow.
1. Installs the **latest currently-published** version of Azurite that is no newer than the
local build (npm and/or Marketplace VSIX) - so a release-day run where the published
version matches the local version still seeds with that (older code generation) release,
while a stale local checkout never ends up testing a downgrade.
.github/workflows/UpgradeCompatibility.yml:12
- The PR description says this workflow deliberately must not run on
pull_request, but this temporary trigger is still enabled, so all four network- and Docker-heavy jobs will run on every PR update after merge. Remove the temporary trigger before merging as the adjacent comment requires.
pull_request:
The top-level summary implied the Marketplace VSIX install is capped at the local build's version like npm/MCR are. It isn't - it's exercised only by the separate lifecycle test, as the requirements table and decision 1 already state.
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 7, 2026 10:15
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/UpgradeCompatibility.yml:12
- Remove this temporary trigger before merging. The PR description and design explicitly say this expensive suite runs only after merges to
mainor on demand; leavingpull_requestenabled runs all four jobs on every PR update and defeats that operational constraint.
pull_request:
The final production stage of both Dockerfile and Dockerfile.Windows ran 'npm ci' with no --omit=dev, reinstalling all 47 devDependencies (typescript, mocha, eslint, autorest, vsce, etc.) on top of the already-built dist/ output. None of these are required by the compiled runtime (dist/src) - verified no devDependency is require()'d there. On Windows this duplicate full install took ~10-11 minutes on top of the builder stage's own ~11 minute install, accounting for most of the Azurite_Windows_Docker job's 25 minute runtime. Also fixes a NODE_ENV=productions typo in Dockerfile.Windows.
Copilot started reviewing on behalf of
Akanksha Jain (jainakanksha-msft)
August 7, 2026 10:56
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 29 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/workflows/UpgradeCompatibility.yml:12
- The temporary
pull_requesttrigger contradicts the PR's stated policy and the adjacent “remove before merging” TODO. If merged as-is, every PR update will run all four expensive compatibility jobs rather than only post-merge/on demand, so remove this trigger before approval.
pull_request:
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Adds a new
tests/upgrade/regression suite plus a dedicated CI workflow that validates Azurite's in-place upgrade / persistence compatibility across releases:.vsix, activate, start all three services, stop them.The suite is intentionally version-agnostic — nothing hardcodes an "old" version number;
versionResolver.tsalways resolves the latest published npm/Marketplace/MCR version at run time, so it keeps working release after release with no maintenance.What's included
tests/upgrade/blobUpgrade.test.ts,queueUpgrade.test.ts,tableUpgrade.test.ts— npm-based cross-version upgrade tests.tests/upgrade/dockerUpgrade.test.ts— Docker/MCR image upgrade test (same bind-mounted volume across image tags).tests/upgrade/vsixLifecycle/— real VS Code extension install/activate/start/stop lifecycle test using@vscode/test-electron.tests/upgrade/utils/:upgradeTarget.ts—UpgradeTargetinterface withNpmProcessTarget/DockerContainerTargetadapters, so npm and Docker scenarios share identical start/stop orchestration.blobUploader.ts/tableValueCodec.ts— shared fixture upload/seed/verify logic, used by both npm and Docker tests so Docker gets full fixture parity (all blob types, all typed table properties).httpProbe.ts,dockerHarness.ts,processHarness.ts,versionResolver.ts,npmVersionInstaller.ts,dataFixtures.ts,integrity.ts..github/workflows/UpgradeCompatibility.yml— runs on push tomain(i.e. after a PR merges) and on demand viaworkflow_dispatch, with jobs for npm (Ubuntu + Windows), VSIX lifecycle (Ubuntu + xvfb), and Docker image upgrade (Ubuntu).test:upgrade,test:upgrade:docker,test:upgrade:vsix,test:upgrade:vsix:published.@vscode/test-electron.docs/designs/2026-08-upgrade-compatibility-testing.md.ChangeLog.mdupdated underGeneral:.Testing
npm run build— clean.npm run test:upgrade— blob/queue/table npm-based upgrade suites pass locally.test:upgrade:dockerandtest:upgrade:vsixare implemented and type-check cleanly but haven't been executed in this dev environment (no Docker CLI / no display available); they're expected to run in the CI workflow'sDockerImageUpgrade_UbuntuandVsixLifecycle_Ubuntujobs (Docker preinstalled,xvfb-runused for the display).Why not run on every PR
The CI workflow deliberately does not trigger on
pull_requestto avoid paying the npm-install / Docker-pull / VS Code download cost on every push to an open PR. It runs once per merge tomain, plus on demand viaworkflow_dispatch.