Add unit tests to aws-typescript #863
Conversation
| import * as pulumi from "@pulumi/pulumi"; | ||
| import * as aws from "@pulumi/aws"; | ||
| import * as awsx from "@pulumi/awsx"; | ||
| import * as infra from "./infra"; |
There was a problem hiding this comment.
It's necessary to separate out the resources into a separate module, because we need to export them to be able to interrogate them in the tests. If we exported them from index.ts then they'd show up as stack outputs, which isn't what we want.
This does make the template more complex, but I can't see a better way to handle it. A potential upside is that it does demonstrate how you can organize your resources into separate modules.
I'm using the generic name infra here for this module. We could pick a name related to the type of resource we're creating. In this case, we're creating a Bucket, so we could name this something like storage instead of infra. However, there's some concern new users might think storage has to do with some kind of Pulumi-specific storage so I'd prefer to keep it a generic name. Also, not all templates are going to be creating storage-related resources, and I'd prefer a generic name that could apply consistently to all of our templates (including the regular typescript template, which doesn't create any cloud resources). Happy to consider alternative generic names beside infra, though.
There was a problem hiding this comment.
This is the standard jest config, configured to support ts-jest.
Note that this file could be jest.config.ts (TypeScript file), but that requires an explicit dependency on ts-node, which we don't currently have for this template (the vendored copy of ts-node in @pulumi/pulumi is used when running the pulumi program), so I've opted to leave it as a JavaScript file. Let me know if you think we should make it a TypeScript file for the TypeScript templates.
justinvp
requested review from
a team,
mjeffryes,
mikhailshilkov and
meagancojocar
There was a problem hiding this comment.
I noticed that the line numbers are wrong on test failures:
I /think/ this is because we include the package source-map-support in pulumi, and that might cause the issue jestjs/jest#10330, but I didn't look further than that.
| // Test helper to convert a Pulumi Output to a Promise. | ||
| // This should only be used in tests. | ||
| function promiseOf<T>(output: pulumi.Output<T>): Promise<T> { | ||
| return new Promise(resolve => output.apply(resolve)); |
There was a problem hiding this comment.
I experimented with custom jest matchers here #868
This keeps the "magic" of Outputs, and avoids the (slightly wrong) conversion to promises here.
There was a problem hiding this comment.
I am not arguing for including these custom matchers in this PR. I was curious to see what it would look like. I think it's something we can follow up with if there are is enough demand to warrant the extra work. We could package the matchers in a library, that can be (optionally) used for writing tests.
Nice catch! I tried uncommenting our use of |
Nice, yeah we should absolutely do that! The version of ts-node we include in our dev dependencies also includes I don't think we need When running via the CLI, we'll usually run through Source map handling is the business of whatever run the program that includes |
| import "jest"; | ||
|
|
||
| // Test helper to convert a Pulumi Output to a Promise. | ||
| // This should only be used in tests. |
There was a problem hiding this comment.
I think we could elaborate a bit more here why this should only be used in tests, maybe something along the lines of:
| // This should only be used in tests. | |
| // This should only be used in tests. Outputs track more information | |
| // than Promises, such as the dependencies between resources. | |
| // https://www.pulumi.com/docs/iac/concepts/inputs-outputs/#outputs |
| }); | ||
|
|
||
| beforeEach(async () => { | ||
| // Dynamically import the infra module. |
There was a problem hiding this comment.
We could mention that this needs to happen after the mocks have been setup, although it is already captured by the use of beforeEach vs beforeAll.
| // Dynamically import the infra module. | |
| // Dynamically import the infra module. This needs to happen | |
| // after the mocks have been setup. |
Would this also fix pulumi/pulumi#9218? |
|
Yes! I'll create a PR for this. |
This change updates the
aws-typescripttemplate to include unit tests using jest. (Once we're happy with this, I'll add the moral equivalent foraws-javascriptin a separate PR).Part of #865