chore(cli): move integ tests to @aws-cdk-testing/cli-integ #23590

rix0rrr · 2023-01-06T12:35:45Z

Clean up the CLI integration tests a bit, by moving them to a separate TypeScript package. This has the following benefits:

More clearly isolates the tests and keeps them separate from the CLI package proper.
Allows normal specification of dependencies of the integration tests themselves, via a proper package.json and Yarn lockfile.
Makes it more clear what reusable files are part of the CLI, vs what reusable files are part of the integration tests.
Makes it more clear/easier to download an old copy of the tests.
Tests that used to be written in bash are now written in TypeScript+Jest, making them easier to maintain.

Also makes the following changes:

Publishes the test artifacts to CodeArtifact, so that we don't need to do crazy shell hijacking shenanigans in order to fake NPM/Maven/NuGet/PyPI repositories from a set of artifacts.
We needed to change how the alpha packages are numbered in a candidate build:
- If the current build was 2.3.4, the candidate build versions would be 2.4.0-rc.0 and 2.4.0-alpha.0.
- However, if the pipeline runs with 2.4.0 actually released, its alpha packages would also be numbered 2.4.0-alpha.0.
- To unambiguously refer to the alpha packages of the candidate version (keeping in mind the naming restrictions of all package managers), we now call those 2.4.0-alpha.999.

This is not the final form of our testing infrastructure. Right now, a fresh CodeArtifact repo is created for every test with all packages in it. In an actual pipeline, we would be better off publishing to a CodeArtifact repo once and all tests pulling from that.

That will be forthcoming in the future.

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

Clean up the CLI integration tests a bit, by moving them to a separate package. This has the following benefits: - More clearly isolates the tests and keeps them separate from the CLI package proper. - Allows normal specification of dependencies of the integration tests themselves, via a proper `package.json` and Yarn lockfile. - Makes it more clear what reusable files are part of the CLI, vs what reusable files are part of the integration tests. - Makes it more clear/easier to download an old copy of the tests. Also makes the following improvement: - Publishes the test artifacts to CodeArtifact, so that we don't need to do crazy shell hijacking shenanigans in order to fake NPM/Maven/NuGet/PyPI repositories from a set of artifacts. This is not the final form of our testing infrastructure. Right now, a fresh CodeArtifact repo is created for every test with all packages in it. In an actual pipeline, we would be better off publishing to a CodeArtifact repo once and all tests pulling from that. That will be forthcoming in the future.

gitpod-io · 2023-01-06T12:35:55Z

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

Naumel · 2023-01-06T12:45:10Z

Started build 516cbb73-c36b-4826-9bee-f5e695df708d

Naumel

None of the questions or comments I have are blocking.
This is a non-trivial amount of files changed, for as far as I can tell, things look good.
Added do-not-merge label.

Naumel · 2023-01-11T12:51:02Z

packages/@aws-cdk-testing/cli-integ/README.md

+* Download the current `@aws-cdk-testing/cli-integ` artifact at `V1`.
+* Determine the previous version `V0` (use `query-github` for this).
+* Download the previous `@aws-cdk-testing/cli-integ` artifact at `V0`.
+* From the `V1` artifact, apply the `V0` patch set.
+* Run the `V0` tests with the `--framework-version` option:


[nit] Perhaps prev/ old or current/new could be more explicit.

I thought about it and I prefer this because it puts you in mind of version numbers.

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

Naumel · 2023-01-11T13:29:02Z

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

+  // this returns a list in descending order, newest releases first
+  // opts for same major version where possible, falling back otherwise
+  // to previous major versions.


[non-blocking] Is there any chance of a release timing forking up functionality?

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

Naumel · 2023-01-11T13:52:44Z

packages/@aws-cdk-testing/cli-integ/lib/resource-pool.ts

+   *
+   * If no such value is currently available, wait until it is.
+   */
+  public async take(): Promise<ILease<A>> {


[non-blocking] Does this have to be public?
Will a consumer ever try to acquire a lease with any other purpose than executing a block?

I like the dual API.

Naumel · 2023-01-11T14:09:06Z

packages/@aws-cdk-testing/cli-integ/lib/staging/maven.ts

+  });
+}
+
+export async function writeMavenSettingsFile(filename: string, login: LoginInformation) {


[high number of file changes, I might just have scrolled past] The "filename" (usageDir) should be removed in a cleanup(), right/ does it?

usageDir is (may be) cleaned up in a separate invocation. The CLI also has two possible APIs:

publish + clean; or

run <cmd>

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

Naumel · 2023-01-11T14:20:59Z

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

+    return XpMutexPool.fromDirectory(path.join(os.tmpdir(), name));
+  }
+
+  private readonly queuedResolvers = new Set<() => void>();


Why a Set and not a Queue (based on name and the FIFO comment from a previous file)?

Hm. There are definitely no FIFO guarantees. Where did you read that?

Naumel · 2023-01-11T14:26:40Z

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


-jest.setTimeout(600_000);
+jest.setTimeout(60 * 60_000); // Includes the time to acquire locks


[non-blocking] If something starts to edge-case, how can we quickly tell we're hitting that scenario?

We'll see timeouts

Naumel · 2023-01-11T14:28:41Z

packages/@aws-cdk-testing/cli-integ/tests/uberpackage/uberpackage.integtest.ts

@@ -1,5 +1,4 @@
-import { withMonolithicCfnIncludeCdkApp } from '../helpers/cdk';
-import { integTest } from '../helpers/test-helpers';
+import { integTest, withMonolithicCfnIncludeCdkApp } from '../../lib';

 jest.setTimeout(600_000);


Does this need updating too?

Nah, it's only 1 test here

Co-authored-by: Naumel <[email protected]>

…ackage

aws-cdk-automation · 2023-01-17T11:52:06Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 4c5f96d
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2023-01-17T13:13:18Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

#31131) ### Reason for this change The cli integration tests cannot use your local version of `aws-cdk-lib`. This can be verified by making your `Stack` construct throw an error upon creation, and watching no CLI integration tests fail, even with `-a`. ### Description of changes Fixed the CLI integration test framework to correctly link the local packages, like it [used to](#23590). ### Description of how you validated changes Manual testing. This isn't something we can add automated tests for. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

aws#31131) ### Reason for this change The cli integration tests cannot use your local version of `aws-cdk-lib`. This can be verified by making your `Stack` construct throw an error upon creation, and watching no CLI integration tests fail, even with `-a`. ### Description of changes Fixed the CLI integration test framework to correctly link the local packages, like it [used to](aws#23590). ### Description of how you validated changes Manual testing. This isn't something we can add automated tests for. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

…k (#31131) ### Reason for this change The cli integration tests cannot use your local version of `aws-cdk-lib`. This can be verified by making your `Stack` construct throw an error upon creation, and watching no CLI integration tests fail, even with `-a`. ### Description of changes Fixed the CLI integration test framework to correctly link the local packages, like it [used to](aws/aws-cdk#23590). ### Description of how you validated changes Manual testing. This isn't something we can add automated tests for. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

rix0rrr requested a review from a team January 6, 2023 12:35

rix0rrr self-assigned this Jan 6, 2023

aws-cdk-automation requested a review from a team January 6, 2023 12:36

github-actions bot added the p2 label Jan 6, 2023

mergify bot added the contribution/core This is a PR that came from AWS. label Jan 6, 2023

rix0rrr changed the title ~~chore(cli): move integ tests to separate package~~ chore(cli): move integ tests to @aws-cdk-testing/cli-integ Jan 6, 2023

aws-cdk-automation previously requested changes Jan 6, 2023

View reviewed changes

rix0rrr added the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label Jan 6, 2023

Merge branch 'main' into huijbers/integ-test-package

556ad56

Naumel added the pr/do-not-merge This PR should not be merged at this time. label Jan 11, 2023

Naumel approved these changes Jan 11, 2023

View reviewed changes

rix0rrr and others added 3 commits January 17, 2023 12:02

Update packages/@aws-cdk-testing/cli-integ/lib/with-aws.ts

18a570f

Co-authored-by: Naumel <[email protected]>

Merge remote-tracking branch 'origin/main' into huijbers/integ-test-p…

f88fdf1

…ackage

Incorporate some comments

4c5f96d

rix0rrr removed the pr/do-not-merge This PR should not be merged at this time. label Jan 17, 2023

mergify bot merged commit 1fefc88 into main Jan 17, 2023

mergify bot deleted the huijbers/integ-test-package branch January 17, 2023 13:13

filletofish mentioned this pull request Jul 2, 2023

feat(scheduler): ScheduleGroup #26196

Merged

comcalvi mentioned this pull request Aug 16, 2024

chore(CLI-integ-tests): cli integ tests cannot use local CDK framework #31131

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(cli): move integ tests to @aws-cdk-testing/cli-integ #23590

chore(cli): move integ tests to @aws-cdk-testing/cli-integ #23590

rix0rrr commented Jan 6, 2023 •

edited

Loading

gitpod-io bot commented Jan 6, 2023

aws-cdk-automation left a comment

Naumel commented Jan 6, 2023

Naumel left a comment

Naumel Jan 11, 2023

rix0rrr Jan 17, 2023

Naumel Jan 11, 2023

rix0rrr Jan 17, 2023

Naumel Jan 11, 2023

rix0rrr Jan 17, 2023

Naumel Jan 11, 2023

rix0rrr Jan 17, 2023

Naumel Jan 11, 2023

rix0rrr Jan 17, 2023

Naumel Jan 11, 2023

rix0rrr Jan 17, 2023

Naumel Jan 11, 2023

rix0rrr Jan 17, 2023

aws-cdk-automation commented Jan 17, 2023

mergify bot commented Jan 17, 2023


		jest.setTimeout(600_000);
		jest.setTimeout(60 * 60_000); // Includes the time to acquire locks

chore(cli): move integ tests to @aws-cdk-testing/cli-integ #23590

chore(cli): move integ tests to @aws-cdk-testing/cli-integ #23590

Conversation

rix0rrr commented Jan 6, 2023 • edited Loading

gitpod-io bot commented Jan 6, 2023

aws-cdk-automation left a comment

Choose a reason for hiding this comment

Naumel commented Jan 6, 2023

Naumel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aws-cdk-automation commented Jan 17, 2023

AWS CodeBuild CI Report

mergify bot commented Jan 17, 2023

rix0rrr commented Jan 6, 2023 •

edited

Loading