feat(e2e): enable windows tests - #2416
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Changes requested
Nice to see Windows finally coming online. One issue that will bite as soon as this runs:
fetch-secrets SHA was only bumped in one of the two call sites
In .github/workflows/e2e-test.yml, only the authorize job's Fetch secrets from Secrets Manager step (line ~46) was bumped to 75989f65f7f193deaf83c237c36572d1a8f800b2. The second invocation in the e2e job — Fetch E2E role from Secrets Manager at line ~104 — is still pinned to the old 31aa3b031a86664e29861d68956e44b07cf21a74:
- name: Fetch E2E role from Secrets Manager
uses: aws/agentcore-devx-devtools/.github/actions/fetch-secrets@31aa3b031a86664e29861d68956e44b07cf21a74That old SHA is the bash-based version — it's the exact one the removed TODO comment called out as not Windows-compatible. The new SHA (aws/agentcore-devx-devtools#5, "swap fetch secret action to use node instead of bash") is what made it Windows-safe. Since the e2e job runs on the matrix including Windows, this step will hit bash on the Windows runner and fail (or at best be flaky, depending on whether Git Bash is on PATH in the image:windows-1.0 runner).
Please bump line 104 to @75989f65f7f193deaf83c237c36572d1a8f800b2 as well (and, while you're in there, the check-authorized-user@31aa3b03... reference on line 51 is fine since authorize runs on ubuntu-latest, but consider keeping all pins in this workflow at the same repo commit for hygiene).
Nit / optional
None of the E2E tests themselves appear to be gated off on Windows, so if any of them shell out to POSIX-only tools or hard-code path separators, expect a wave of new failures on first run. Not a blocker for this PR, just a heads-up — worth watching the first Windows run closely.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2416 +/- ##
=========================================
Coverage 97.25% 97.25%
=========================================
Files 612 612
Lines 41019 41019
=========================================
Hits 39893 39893
Misses 1126 1126 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2bd121b to
daf657c
Compare
|
Claude Security Review: no high-confidence findings. (run) |
Problem
Windows tests are not running on the e2e branch since the fetch secrets action was not windows compatible.
This has been fixed in aws/agentcore-devx-devtools@75989f6.
Solution