feat: make the integ test harness work on Windows - #1854
Open
dgandhi62 wants to merge 8 commits into
Open
Conversation
dgandhi62
changed the title
aws-cdk-automation
enabled auto-merge
August 19, 2026 20:56
dgandhi62
changed the title
iliapolo
requested changes
Aug 20, 2026
| // still. Install every distinct package set only once per machine and | ||
| // junction it into the test directory. | ||
| const sharedNodeModules = await sharedPackageSetInstall(fixture, packages); | ||
| fs.symlinkSync(sharedNodeModules, path.join(fixture.integTestDir, 'node_modules'), 'junction'); |
Contributor
There was a problem hiding this comment.
Interesting. Lets make use of it on linux as well! We've seen network errors due to multiple npm install processes and I remember also seeing some slow installation in our CodeBuild canary runs.
Contributor
There was a problem hiding this comment.
This was one of the biggest changes for bringing down Windows integ test times, since npm install is so much less efficient. Concur on running it on Linux. @dgandhi62 thanks for the changes, can you show us a workflow run using the latest commit to validate it works as expected (tests pass and shorter test times)?
Contributor
Author
There was a problem hiding this comment.
Running it with the latest code now. Will link it here once done
Contributor
Author
There was a problem hiding this comment.
iankhou
reviewed
Aug 21, 2026
| await context.library.initializeDotnetPackages(context.integTestDir); | ||
| await shell.shell(['cdk', 'synth']); | ||
| }))); | ||
| })), 300_000); |
Contributor
There was a problem hiding this comment.
For each instance where we adjust test timeouts, please verify in the workflow run whether it needs this much time.
Contributor
Author
There was a problem hiding this comment.
I can reduce the timeouts in some to make it closer to the test times. Depending on the urgency of this pr, we can have a separate one to make efficiency improvements in the tests themselves
Contributor
Author
There was a problem hiding this comment.
For the ask of updating timeouts based on test times, that is done. If the ask is to identify test improvements to reduce times, that might have to be a separate pr
Contributor
There was a problem hiding this comment.
No ask for the latter. Reducing the timeouts is OK.
dgandhi62
force-pushed
the
windows-integ-1-harness
branch
from
August 21, 2026 15:48
4e33fc8 to
a8f331f
Compare
The integ test harness and several suites assume a POSIX environment, so they cannot run on a Windows runner. This makes them platform-portable without changing behaviour on Linux. - spawn TTY processes through the shell, and widen the ConPTY terminal so long prompts are not wrapped before they are matched - match prompts against ConPTY screen-buffer output - spawn npm through the node interpreter rather than relying on bin shims - share one npm install across tests, which dominates runtime on Windows - fix path handling in the watch tests and search all stage assemblies for the nested template - give the init suites an explicit 5 minute timeout; they previously ran on the 60s suite default, which is not enough for Maven, NuGet or Go module downloads No Windows jobs run yet; enabling those is a follow-up.
The shared install added for Windows applies just as well everywhere:
every test asks for the same handful of packages at the same resolved
versions, so installing per test is duplicated work. On Linux that shows
up as many concurrent `npm install` processes, which is a known source of
ECONNRESET failures, and as slow installs in the CodeBuild canary runs.
- drop the win32 gate, and pick the symlink type per platform ('junction'
on Windows, where a 'dir' symlink needs elevation; ignored on POSIX)
- keep per-test installs when REPO_ROOT rewrites a package to a local
directory: the cache is keyed on the requested package set, and a
directory path does not change when its contents are rebuilt, so
sharing there would serve stale code. No package installed here is
currently a workspace of this repo, so this is a guard, not a fix
- coordinate through the existing XpMutex instead of a hand-rolled lock
directory. It reclaims a lock whose owner has died by checking pid
liveness, rather than waiting out a timeout: previously a worker killed
mid-install left a lock nothing would release, so every other test on
the machine waited out the 30 minute deadline and failed
Keying on the package set is safe because `requestedVersion()` always
resolves to an exact version before it reaches the installer.
Addresses review feedback on the shared-install block.
The cross-process mutex guarding the shared npm install represents a lock as a file: acquire by exclusively creating it, release by unlinking it. This assumes POSIX deletion semantics, where the only "can't create" signal is EEXIST and the only "can't read" signal is ENOENT. Windows differs. A lock file that another process still has open, or that was just unlinked, enters a "delete pending" state: it lingers in the directory but open()/read() against it fail with EPERM/EACCES. Under the heavy startup contention the shared install creates (every jest worker races for the same lock), tryAcquire() hit EPERM, fell into the `code !== 'EEXIST'` branch, and rethrew a fatal error. On the Windows integ runner this took down every test in the suite with an identical 'EPERM: operation not permitted, open ...cdk-integ-shared-install...mutex'. Treat EPERM/EACCES the same as the POSIX signals: on exclusive create they mean "held or mid-transition, back off and retry" (like EEXIST); on read they mean "not readable, treat as gone" (like ENOENT). Add a short sleep before retrying so a persistent delete-pending window does not busy-spin. POSIX behavior is unchanged: EPERM does not occur on this path there, so the new branches are inert on Linux and macOS. Also bump the init-typescript-app integ test timeouts (300s->600s, 180s->300s) to account for the slower Windows runners.
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
4 participants
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.
This pr is created for maintaining separation of concerns from here - #1781
Here, we make modifications to the testing harness, since a lot of the older code was linux-specific and did not run properly on Windows. The code to enable the windows tests will be a follow up.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license