Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(doc): update the contributing guide with more details #25321

Merged
merged 10 commits into from
Apr 27, 2023
104 changes: 104 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,110 @@ $ cd packages/aws-cdk
$ yarn watch & # runs in the background
```

#### Verify your fix by deployment

If your PR updates a specific library, you will need to write a simple CDK application and make sure it synthesizes and
deploys correctly. For example, if you modify files under `packages/aws-cdk-lib/aws-eks`, you can write a simple CDK app in typescript to verify its behavior:
pahud marked this conversation as resolved.
Show resolved Hide resolved


```console
$ cd packages/@aws-cdk-testing/framework-integ/test/aws-eks/test
```

Create a `sample.ts` like this:

```ts
import {
App, Stack,
aws_eks as eks,
aws_ec2 as ec2,
} from 'aws-cdk-lib';
import { getClusterVersionConfig } from './integ-tests-kubernetes-version';

const app = new App();
const env = { region: process.env.CDK_DEFAULT_REGION, account: process.env.CDK_DEFAULT_ACCOUNT };
const stack = new Stack(app, 'my-test-stack', { env });

const cluster = new eks.Cluster(stack, 'Cluster', {
vpc,
...getClusterVersionConfig(stack),
defaultCapacity: 0,
});
```

Run `yarn watch` or `npx tsc --watch` in a separate terminal to compile `sample.ts` to `sample.js`:

```console
$ cd packages/@aws-cdk-testing/framework-integ
$ yarn watch
or
$ npx tsc --watch
```

Make sure you have configured [AWS CLI with AWS Authentication](https://docs.aws.amazon.com/cdk/v2/guide/getting_started.html#getting_started_auth) as we will deploy it in our AWS account.

Deploy the sample app:

```console
$ cd packages/@aws-cdk-testing/framework-integ
$ npx cdk -a test/aws-eks/test/sample.js diff
$ npx cdk -a test/aws-eks/test/sample.js deploy
```

This allows you to iterate your development and ensure a minimal sample app successfully deploy as you expect.
You have the freedom to interact with it just as a common CDK app such as viewing differences with `npx cdk diff`
or pass context variables with `npx cdk deploy -c`. You can rapidly iterate your testing with repeated deployments
by importing existing resource such as existing VPC. This can save a lot of time and help you focus on the core changes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This allows you to iterate your development and ensure a minimal sample app successfully deploy as you expect.
You have the freedom to interact with it just as a common CDK app such as viewing differences with `npx cdk diff`
or pass context variables with `npx cdk deploy -c`. You can rapidly iterate your testing with repeated deployments
by importing existing resource such as existing VPC. This can save a lot of time and help you focus on the core changes.
This allows you to iterate your development and ensures a minimal sample app successfully deploys as expected.
You have the freedom to interact with it just as a common CDK app such as viewing differences with `npx cdk diff`
or pass context variables with `npx cdk deploy -c`. You can rapidly iterate your testing with repeated deployments
by importing existing resource such as an existing VPC. This can save a lot of time and help you focus on the core changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DONE


```ts
const vpc = ec2.Vpc.fromLookup(stack, 'Vpc', { isDefault: true }),
```

As this is for testing only, do not commit `sample.ts` and `sample.js` to your PR branch.

Alternatively, you can just write this test as a new integration test like `integ.my-test.ts` and deploy it
with `yarn integ` and generate snapshots for you. This would be very useful when you need to publish a new
integration test:
pahud marked this conversation as resolved.
Show resolved Hide resolved

```console
$ cd packages/@aws-cdk-testing/framework-integ
$ yarn integ test/aws-eks/test/integ.my-test.js --no-clean --update-on-failed
```

After verifying your work with a simple deployment as above, you need to ensure your hack can pass all existing
pahud marked this conversation as resolved.
Show resolved Hide resolved
unit tests and integ tests and fix them if necessary.

Run all the unit tests for a specific module(e.g. aws-eks):

```console
$ cd packages/aws-cdk-lib
$ yarn test aws-eks
```

Or run a specific unit test

```console
$ cd packages/aws-cdk-lib
$ npx jest aws-eks/test/name.test.js
```

Run all integ tests for a specific module(e.g. aws-eks):

```console
$ cd packages/@aws-cdk-testing/framework-integ
$ yarn integ --directory test/aws-eks/test
```

Or run a specific integ test:

```console
$ yarn integ --directory test/aws-eks/test/integ.name.js
```

See the [integration test guide](./INTEGRATION_TESTS.md) for a more complete guide on running
CDK integration tests.


### Step 4: Pull Request

* Create a commit with your changes and push them to a
Expand Down