DO NOT MERGE - feat(cli-integ): also run integ test suites on Windows - #1781
Draft
iankhou wants to merge 21 commits into
Draft
DO NOT MERGE - feat(cli-integ): also run integ test suites on Windows#1781iankhou wants to merge 21 commits into
iankhou wants to merge 21 commits into
Conversation
Adds windowsTestRunsOn to the integ workflow. When set, every integ suite (integ_cli, integ_toolkit-lib, integ_telemetry, integ_init-templates, integ_tool-integrations) is instantiated a second time on a Windows runner, suffixed with _windows, to catch platform-specific regressions in path handling and subprocess spawning. The prepare/build job stays on Linux, so there is no repo checkout on the Windows runners; they only download the built artifacts and run the suites. Steps run under Git Bash so the shared bash step scripts work unchanged.
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
pm2's fork mode cannot launch the global `verdaccio` bin on Windows because it is a `.cmd` shim, not a Node script, so the registry never came up and every integ job failed at the publish step. Resolve the JS entrypoint and start it with an explicit `--interpreter node`, which works on all platforms.
Three POSIX-isms in the test harness broke every Windows integ run: - cloneDirectory() shelled out to `rm -rf`/`mkdir -p`/`cp -R`, which do not exist under cmd.exe. Replaced with fs.promises equivalents. - addToShellPath() split and joined PATH with ':'; on Windows the delimiter is ';', so prepending the CLI bin dir corrupted the PATH and `cdk` was never found. Use path.delimiter. - A few tests shelled out to `rm`, `cat` (with globs), and `diff`; replaced with portable fs-based implementations.
spawnSync('npm', ...) fails on Windows because npm is a .cmd shim, not an
executable; JSON.parse then chokes on the undefined stdout. Resolve the npm
JS entrypoint and invoke it through process.execPath, matching how the rest
of the harness already calls npm.
The portable replacement for `cat cdk.out/assembly-*-stage/*StackInStage*` picked the first directory matching assembly-*-stage, which can be the bundling stage that contains no StackInStage template. Search recursively for the full relative path instead, mirroring the original glob semantics.
…on venv layout
- ecrPublicLogin passed the ECR password as a literal `${ECR_PASSWORD}`,
which cmd.exe does not expand, so `docker login` sent the literal string
and got a 400. Use `%ECR_PASSWORD%` on Windows. This failed every test
using the default fixture (~200 tests) since the fixture logs in at setup.
- init-python looked for the virtualenv binaries in `.venv/bin`; on Windows
virtualenv creates `.venv/Scripts`. Also use path.delimiter for PATH.
… Windows
The %ECR_PASSWORD% expansion fix got docker login past authentication, but
storing the credential then failed with 'error storing credentials - err:
exit status 1, out: The stub received bad data' from the wincred helper,
which does not handle ~40 parallel logins from concurrent test workers.
Write {"credsStore": ""} into the per-test DOCKER_CONFIG dir so credentials
go to the config file, matching Linux runner behavior.
The init suites (java, go, python, csharp, fsharp, javascript, typescript-lib) ran with jest's suite default of 60s. On Windows runners a `cdk init` plus toolchain build (mvn package, go test, pip install) takes 3-5 minutes, so every test timed out. Worse, jest keeps the timed-out test body running while it starts the retry, and the zombie's subsequent commands fail confusingly. init-typescript-app already used 300s; apply the same to the rest.
Disabling credsStore in the config file was not enough: docker on Windows
auto-detects the wincred helper and still fails to store the ~1.5KB ECR
token ('The stub received bad data'). Skip `docker login` entirely on
Windows and write the auths entry into the per-test DOCKER_CONFIG
config.json ourselves - the same end state docker login produces on the
Linux runners.
…nners Defender real-time scanning hooks every file write. The integ tests are dominated by npm installs and toolchain builds that create tens of thousands of small files, making Windows jobs 3-10x slower than Linux (e.g. tool-integrations: 19-29 min vs 2.5 min). Exclude the workspace, temp, npm and toolcache directories from scanning; the runner VM is ephemeral and job-isolated so this carries no persistent risk.
…app timeouts
- GitHub Windows runners run Docker in Windows-containers mode and cannot
pull or build Linux images ('no matching manifest for windows/amd64', or
no daemon pipe at all). Skip the 13 tests that build/run Linux images on
Windows via CDK_INTEG_SKIP_TESTS; they retain full coverage on Linux.
- The typescript-version matrix tests in init-typescript-app still used the
60s suite default and timed out on Windows; the templated init tests hit
300s when several npm installs run concurrently. Raise to 300s/600s.
A/B timing between runs with and without the Defender exclusion showed no measurable difference - runner images evidently already handle it. Replace it with a Dev Drive (ReFS VHDX): all test fixtures live under os.tmpdir(), so pointing TEMP/TMP at the Dev Drive moves the npm-install-heavy file churn onto the faster filesystem. windows-latest is Server 2025 (build 26100), which supports Format-Volume -DevDrive natively.
TEMP-only Dev Drive placement showed 25-35% job speedups on npm-heavy suites but left the npm cache on the OS disk, where every npm invocation in the job (global verdaccio install, per-test installs) still pays NTFS tax. Point npm_config_cache at the Dev Drive as well, and grow the dynamically-allocated VHDX to 40GB to fit cache plus concurrent fixtures.
These tests do not have docker in the name but build Linux images as a side effect and fail on Windows runners the same way as the named docker tests: python lambda bundling (sam/build-python3.12 image), SAM metadata asset bundling, a DockerImageAsset in the session-tags fixture stack, and a docker-app deploy from a copied cloud assembly. Their hung docker builds were also occupying jest workers and pushing unrelated tests in the same shard past the 1200s harness timeout.
Windows cli shards run 30-80 minutes; tests still executing past the 1-hour default session expiry fail with '403 the security token included in the request is expired', including Atmosphere release calls. Request a 4-hour session on Windows jobs, matching the non-Atmosphere path. Takes effect only if the OIDC role's maximum session duration allows it, which this change also verifies.
The Atmosphere OIDC role's MaxSessionDuration is 1 hour: requesting 4
hours fails the assume-role call itself ('The requested DurationSeconds
exceeds the MaxSessionDuration set for this role'). Revert to the 1-hour
request and document why. Long-running Windows jobs will still 403 on
tests running past expiry; fixing that needs a MaxSessionDuration bump on
the role or credential refresh in the Atmosphere client.
Contributor
|
Question - can I merge this? :) |
node-pty's ConPTY backend resolves the target with SearchPath, which only finds real executables, not the .cmd shims npm generates for CLI entrypoints. Spawning 'cdk' this way fails with 'File not found:', breaking every test that needs a TTY (cdk destroy/import prompts, tty-app). Route the command through cmd.exe /c, mirroring what Process.spawn does with shell: true.
The watch tests spawned 'cdk' directly with child_process.spawn, which
cannot start npm .cmd shims on Windows ('spawn cdk ENOENT'); the tests
then sat in waitForOutput until the 120s poll timeout. Spawn through a
shell on win32 (new spawnWatch helper), kill the resulting process tree
with taskkill so the watcher does not outlive the test, and replace the
POSIX-only 'touch' with fs.utimesSync.
Two more tests build Linux docker images as a side effect and cannot pass on Windows-containers runners: 'test resource import with construct that requires bundling' (INCLUDE_NODEJS_FUNCTION_LAMBDA forces docker bundling of a NodejsFunction) and 'hotswap deployment supports Bedrock AgentCore Runtime' (builds a linux/arm64 DockerImageAsset). The session-tags skip name contains commas, which the comma-separated CDK_INTEG_SKIP_TESTS env var splits into fragments that match nothing — the test ran (and failed) despite being listed. Write the skip list to a newline-separated file and point CDK_INTEG_SKIP_TESTS_FILE at it instead.
Every test installs aws-cdk-lib into its own temp directory. Writing out that package's tens of thousands of files takes ~3s on Linux but 10-12 minutes on Windows, and all ~15 jest workers doing it concurrently is the bulk of the Windows wall-clock (the same install takes 41s when a single worker runs it alone). On win32, install each distinct package set once per machine into a shared directory (workers coordinate through an atomic mkdir lock) and junction it into the test directory. rimraf now removes links without recursing into them so test cleanup cannot delete the shared install.
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.
Do not merge this. This is a POC.
Adds windowsTestRunsOn to the integ workflow. Every integ suite (currently running on Ubuntu) will now also run on GitHub's
windows-latest.Windows jobs download the built artifacts and run the suites using Git Bash.
Made modifications to the testing harness, since a lot of code was linux-specific and did not run properly on Windows.
For Windows tests, we excluded 17 integration tests that use Linux docker images, which don't work on
windows-latest's kernel. We also use Dev Drive, a Windows 11 feature that helps to optimize file I/O operations that would otherwise take a longer time on Windows (because of NTFS and synchronous Windows Defender scanning).Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license