fix(integ-runner): catch snapshot errors, treat --from-file as command-line#20523
fix(integ-runner): catch snapshot errors, treat --from-file as command-line#20523mergify[bot] merged 5 commits intomasterfrom
--from-file as command-line#20523Conversation
|
|
| }); | ||
| this.cdkOutDir = options.integOutDir ?? `${CDK_OUTDIR_PREFIX}.${testName}`; | ||
| this.cdkApp = `node ${parsed.base}`; | ||
| this.cdkOutDir = options.integOutDir ?? `${CDK_OUTDIR_PREFIX}.${this.testName}`; |
There was a problem hiding this comment.
this.testName needs to be the baseTestName from testNameFromFile
There was a problem hiding this comment.
And actually can you just add baseTestName to the TestNameParts. Maybe call it stableTestName? The testName (really the testDisplayName) is something that changes depending on where you run the test from, but we also need a stable test name that doesn't change to use here
There was a problem hiding this comment.
Yes I'm running into some trouble with the other change I'm trying to make here as well. They seem to come together.
…and-line Snapshot errors --------------- The constructor of `IntegSnapshotRunner` calls `loadManifest()`, which on my computer happened to fail and stopped the entire test suite because this error happened outside the `try/catch` block. Move it inside the try/catch block, needed a bit of refactoring to make sure we could still get at the test name. `--from-file` ------------- Instead of having `--from-file` require a JSON file with its own structure, interpret it as a text file which gets treated exactly the same as the `[TEST [..]]` arguments on the command line. This still allows for the `--exclude` behavior by setting that flag on the command-line. Refactoring ----------- Moved the logic around determining test names and directories into a class (`IntegTest`) which is a convenience class on top of a static data record (`IntegTestInfo`). We pass the data record to worker threads.
fb10189 to
2007431
Compare
--from-file as command-line
Print snapshot validations that are running for a long time (1 minute). This helps diagnose what is stuck, if anything is. On my machine, it was tests using Docker because there was some issue with it, and this lost me a day. Also change the test reporting formatting slightly, and catch errors during actual test running.
corymhall
left a comment
There was a problem hiding this comment.
Looks great! Love these improvements ❤️
| * - The suite name | ||
| * - The absolute filename | ||
| */ | ||
| public matches(name: string) { |
| readonly cdk?: ICdk; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Is this comment left over?
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Snapshot errors
The constructor of
IntegSnapshotRunnercallsloadManifest(), whichon my computer happened to fail and stopped the entire test suite
because this error happened outside the
try/catchblock. Same forthe integ runner itself.
Move it inside the try/catch block, needed a bit of refactoring to
make sure we could still get at the test name.
--from-fileInstead of having
--from-filerequire a JSON file with its ownstructure, interpret it as a text file which gets treated exactly
the same as the
[TEST [..]]arguments on the command line.This still allows for the
--excludebehavior by setting that flagon the command-line.
Also be very liberal on the pattern (file name or test name or display
name) that we accept, encoded in the
IntegTest.matches()class.Refactoring
Moved the logic around determining test names and directories into a
class (
IntegTest) which is a convenience class on top of a static datarecord (
IntegTestInfo). The split between data and logic is so that wecan pass the data to worker threads where we can hydrate the helper
class on top again. I tried to be consistent: in memory, all the fields are
with respect to
process.cwd(), so valid file paths in the currentprocess. Only when they are passed to the CLI wrapper are the paths
made relative to the CLI wrapper directory.
Print snapshot validations that are running for a long time (1 minute).
This helps diagnose what is stuck, if anything is. On my machine, it
was tests using Docker because there was some issue with it, and this
lost me a day. Also change the test reporting formatting slightly.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license