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
use test environments for codegen tests #15884
base: master
Are you sure you want to change the base?
Conversation
Changelog[uncommitted] (2024-04-15) |
Currently these tests always just use the directory in the repository tree to run. This isn't great for a couple of reasons: - The tests write files into the repo tree (these are .gitignored, but still large and aren't cleaned up after the test finished) - Tests running in parallel can potentially use the same directory. We have a nice helper that creates new test environments and cleans them up at the end of the tests. Use that here for more test isolation. Also remove the t.Run() in the inner loop. For a lot of these tests it's not possible to run only one bit at a time (which is also why there is an ordering enforced). It's better to just make them run in the outer test loop to avoid confusion when trying to rerun only part of the test.
3a33af2
to
3b0b261
Compare
@@ -574,8 +585,8 @@ func TestSDKCodegen(t *testing.T, opts *SDKCodegenOptions) { // revive:disable-l | |||
files, err := GeneratePackageFilesFromSchema(schemaPath, opts.GenPackage) | |||
require.NoError(t, err) | |||
|
|||
if !RewriteFilesWhenPulumiAccept(t, dirPath, opts.Language, files) { | |||
expectedFiles, err := LoadBaseline(dirPath, opts.Language) | |||
if !RewriteFilesWhenPulumiAccept(t, e.CWD, opts.Language, files) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this break the PULUMI_ACCEPT flow? Because we're going to rewrite the files in the temp dir?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah it does. We probably need to re-export these files.
The latest commit fixes that.
Currently these tests always just use the directory in the repository tree to run. This isn't great for a couple of reasons:
We have a nice helper that creates new test environments and cleans them up at the end of the tests. Use that here for more test isolation.
Also remove the t.Run() in the inner loop. For a lot of these tests it's not possible to run only one bit at a time (which is also why there is an ordering enforced). It's better to just make them run in the outer test loop to avoid confusion when trying to rerun only part of the test.
(I originally noticed this for the Bazel hackathon project. Bazel only creates symlinks to files and then the
yarn install
creates the node_modules folder within that folder with the symlinks. However tsc then looks for the node_modules folder in the folder the symlink of the source files point to. So that's an ulterior motive for this change, but I think the cleanup is worth it either way)