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

feat(remodel): make tests more directory agnostic #24359

Conversation

MrArnoldPalmer
Copy link
Contributor

Adds a bunch of changes to various unit tests to make them work regardless of their location within the repository as long as they are relative to the files they reference. Before some of these depended on the current working directory being different than they will be in aws-cdk-lib. Some of these still need to get update but will be easier to do by hand.

Additionally adds a bunch of missing build steps to aws-cdk-lib package.json.cdk-build.pre. These steps were all in individual modules before and built artifacts for various tests.

Also adds the runBuild function back into remodel. This builds everything, which unfortunately we need to do to be able to generate the alpha packages` individual pkg gen requires a bunch of the build tools and the packages that are generated themselves to actually be built first.


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

but using compilerOptions.outDir messages with non-compiled files like
.json. Just not worth it =(.
@MrArnoldPalmer MrArnoldPalmer marked this pull request as ready for review February 28, 2023 00:40
@aws-cdk-automation aws-cdk-automation requested a review from a team February 28, 2023 00:40
@github-actions github-actions bot added the p2 label Feb 28, 2023
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Feb 28, 2023
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

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.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

@MrArnoldPalmer MrArnoldPalmer changed the title Feat/make tests dir generic feat(remodel): make tests more directory agnostic Feb 28, 2023
if (!_cdkVersion) {
throw new Error('Unable to determine CDK version');
}
}
return _cdkVersion;
}

function findParentPkgJson(dir: string, depth: number = 1, limit: number = 5): { version: string; } {
Copy link
Contributor

Choose a reason for hiding this comment

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

for this one, and findParentTsConfigPath, if we need them in another test, let's make sure to extract to a shared utility. I am mostly just writing this comment so I remember about it, I don't think it's necessary now.

Co-authored-by: Madeline Kusters <80541297+madeline-k@users.noreply.github.com>
@MrArnoldPalmer MrArnoldPalmer merged commit d35fc8c into aws:feat/repo-restructure Feb 28, 2023
@aws-cdk-automation
Copy link
Collaborator

The pull request linter fails with the following errors:

❌ Features must contain a change to a README file.
❌ Features must contain a change to an integration test file and the resulting snapshot.

PRs must pass status checks before we can provide a meaningful review.

If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing Exemption Request and/or Clarification Request.

@MrArnoldPalmer MrArnoldPalmer deleted the feat/make-tests-dir-generic branch April 26, 2023 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants