chore(integ-tests): decouple CLI and Library sources, improve documentation #425

rix0rrr · 2025-04-29T14:05:46Z

We used to make assumptions in the integ tests that the CLI and library versions were the same unless specified otherwise, and that they both would come from the same repo.

These assumptions are no longer true: the CLI and the library have their own version numbers, and only the CLI can come from a source repo (or at least, that's how it works in practice).

Tear apart ReleasePackageSource and RepoPackageSource into (currently):

CliNpmSource / CliRepoSource
LibraryNpmSource

Make a more clear naming distinction between the part that runs in the runner and the part that runs in the test.

Explain in documentation what the options are, and how the use of the given component is achieved at runtime.

Make the argument names and default values a little more sensible given the new source locations.

CHANGES:

Add --cli-version to mirror --framework-version.
Default to --auto-source if no CLI version given.
Add -F alias to specify a single test file.
Resolve the framework version immediately.
Deprecate --use-source and replace with --cli-source.
Deprecate --auto-source and replace with --cli-source=auto.
Change to a different yargs version and twiddle all the options slightly differently to enable strict parsing mode.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

codecov-commenter · 2025-04-29T14:19:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.25%. Comparing base (1d4fd87) to head (257f7ce).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #425      +/-   ##
==========================================
+ Coverage   79.18%   79.25%   +0.06%     
==========================================
  Files          54       54              
  Lines        6895     6897       +2     
  Branches      772      772              
==========================================
+ Hits         5460     5466       +6     
+ Misses       1417     1413       -4     
  Partials       18       18

Flag	Coverage Δ
suite.unit	`79.25% <ø> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2025-04-30T08:44:00Z

Total lines changed 1043 is greater than 1000. Please consider breaking this PR down.

mrgrain · 2025-05-05T10:41:29Z

.projenrc.ts

+          statements: 25,
+          lines: 25,


Not loving this. Can you at least explain why these are lowered?

Yeah. There's a bunch of new code that involves calling NPM and making symlinks on disk, that doesn't have unit tests because it's either awkward to test or introduces sources of network calls and latency that we don't really want in unit tests (imo).

I figured since this is for the integ tests, it is basically tested as part of every PR already.

mrgrain · 2025-05-05T11:31:37Z

packages/@aws-cdk-testing/cli-integ/lib/cli/run-suite.ts

-  if (args['use-source'] || args['auto-source']) {
-    if (args['framework-version']) {
-      throw new Error('Cannot use --framework-version with --use-source');
+  for (const flagAlias of ['cli-source', 'use-source'] as const) {


I think this gives --use-source precedence over --cli-source if both are defined.

cliSource is of type UniqueOption<T> which will throw an error if it's set more than once.

mrgrain · 2025-05-05T11:32:43Z

packages/@aws-cdk-testing/cli-integ/lib/package-sources/find-root.ts

+import { findUp } from '../files';
+
+/**
+ * Find the root directory of the repo from the current directory


Might help explaining why you are looking for release.json

mrgrain · 2025-05-05T11:33:05Z

packages/@aws-cdk-testing/cli-integ/lib/package-sources/find-root.ts

+/**
+ * Find the root directory of the repo from the current directory
+ */
+export async function autoFindRoot() {


Maybe this? Because it could also be package root.

Suggested change

export async function autoFindRoot() {

export async function autoFindRepoRoot() {

mrgrain · 2025-05-05T11:33:40Z

packages/@aws-cdk-testing/cli-integ/lib/package-sources/library-npm-source.ts

+  }
+
+  public assertJsiiPackagesAvailable(): void {
+    // FIXME: Always a no-op.


Is this a FIXME then if it's always a no-op?

mrgrain · 2025-05-05T11:35:38Z

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cdk-deploy-with-role.integtest.ts

Why is this test removed?

Because the test started with

if (fixture.packages.majorVersion() !== '1') { return; // Nothing to do }

And so it was always a no-op. We haven't run this test for 2 years.

…tation We used to make assumptions in the integ tests that the CLI and library versions were the same unless specified otherwise, and that they both would come from the same repo. These assumptions are no longer true: the CLI and the library have their own version numbers, and only the CLI can come from a source repo (or at least, that's how it works in practice). Tear apart `ReleasePackageSource` and `RepoPackageSource` into (currently): - `CliNpmSource` / `CliRepoSource` - `LibraryNpmSource` Make a more clear naming distinction between the part that runs in the runner and the part that runs in the test. Explain in documentation what the options are, and how the use of the given component is achieved at runtime. Make the argument names and default values a little more sensible given the new source locations. CHANGES: - Add `--cli-version` to mirror `--framework-version`. - Default to `--auto-source` if no CLI version given. - Add `-F` alias to specify a single test file. - Resolve the framework version immediately. - Deprecate `--use-source` and replace with `--cli-source`. - Deprecate `--auto-source` and replace with `--cli-source=auto`. Signed-off-by: github-actions <github-actions@github.com>

rix0rrr temporarily deployed to integ-approval April 29, 2025 14:05 — with GitHub Actions Inactive

aws-cdk-automation requested a review from a team April 29, 2025 14:05

aws-cdk-automation enabled auto-merge April 29, 2025 14:05

github-actions bot added the p2 label Apr 29, 2025

rix0rrr force-pushed the huijbers/integ-sources branch from 7c3c3d6 to 9ecb5c8 Compare April 29, 2025 14:11

rix0rrr temporarily deployed to integ-approval April 29, 2025 14:11 — with GitHub Actions Inactive

rix0rrr self-assigned this Apr 29, 2025

rix0rrr had a problem deploying to run-tests April 29, 2025 17:36 — with GitHub Actions Failure

rix0rrr force-pushed the huijbers/integ-sources branch from 9ecb5c8 to a01be6a Compare April 30, 2025 08:43

rix0rrr temporarily deployed to integ-approval April 30, 2025 08:43 — with GitHub Actions Inactive

rix0rrr had a problem deploying to run-tests April 30, 2025 08:53 — with GitHub Actions Failure

rix0rrr temporarily deployed to run-tests May 5, 2025 09:56 — with GitHub Actions Inactive

mrgrain reviewed May 5, 2025

View reviewed changes

mrgrain disabled auto-merge May 5, 2025 11:37

mrgrain approved these changes May 5, 2025

View reviewed changes

rix0rrr force-pushed the huijbers/integ-sources branch from 198dcf9 to 257f7ce Compare May 5, 2025 12:02

rix0rrr temporarily deployed to integ-approval May 5, 2025 12:02 — with GitHub Actions Inactive

rix0rrr enabled auto-merge May 5, 2025 12:07

rix0rrr temporarily deployed to run-tests May 5, 2025 12:14 — with GitHub Actions Inactive

rix0rrr added this pull request to the merge queue May 5, 2025

Merged via the queue into main with commit 7a950ef May 5, 2025
20 checks passed

rix0rrr deleted the huijbers/integ-sources branch May 5, 2025 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(integ-tests): decouple CLI and Library sources, improve documentation #425

chore(integ-tests): decouple CLI and Library sources, improve documentation #425

Uh oh!

rix0rrr commented Apr 29, 2025 •

edited

Loading

Uh oh!

codecov-commenter commented Apr 29, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Apr 30, 2025

Uh oh!

mrgrain May 5, 2025

Uh oh!

rix0rrr May 5, 2025

Uh oh!

mrgrain May 5, 2025

Uh oh!

rix0rrr May 5, 2025

Uh oh!

mrgrain May 5, 2025

Uh oh!

mrgrain May 5, 2025

Uh oh!

mrgrain May 5, 2025

Uh oh!

mrgrain May 5, 2025

Uh oh!

rix0rrr May 5, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	export async function autoFindRoot() {
	export async function autoFindRepoRoot() {

chore(integ-tests): decouple CLI and Library sources, improve documentation #425

chore(integ-tests): decouple CLI and Library sources, improve documentation #425

Uh oh!

Conversation

rix0rrr commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-commenter commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Apr 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rix0rrr commented Apr 29, 2025 •

edited

Loading

codecov-commenter commented Apr 29, 2025 •

edited

Loading