chore(toolkit): tests don't exit #33427
Conversation
On GitHub Actions the `jest` test run doesn't exit because it is being kept alive. The reason is that we create a `CloudWatchLogMonitor` in one test, which keeps on scheduling a timer with `setTimeout()` which keeps the Node process alive. I considered `unref()`fing the timer, but it seems a smaller and therefore safer change to take the `CloudWatchLogMonitor` and deactivate it.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #33427 +/- ##
=======================================
Coverage 80.90% 80.90%
=======================================
Files 236 236
Lines 14261 14261
Branches 2493 2493
=======================================
Hits 11538 11538
Misses 2438 2438
Partials 285 285
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| })); | ||
|
|
||
| // Deactivate the cloudWatchLogMonitor that we created, otherwise the tests won't exit | ||
| (deploySpy.mock.calls[0][2] as any).cloudWatchLogMonitor?.deactivate(); |
There was a problem hiding this comment.
Should this be in a finally block then?
There was a problem hiding this comment.
We can only get a reference to the right object if the get far enough that the call succeeds and we can dig the object out of the call sequence.
So if we break this test the experience won't be great... but in that case we're broken anyway
There was a problem hiding this comment.
Ok, would be nice to make the broken experience better though. Won't insist.
There was a problem hiding this comment.
I don't think the way cloud watch logs monitoring is currently implemented makes a lot of sense for the toolkit. I'm currently making my way through the deploy action and can take a look.
|
Thank you for contributing! Your pull request will be updated from main 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 main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
4 participants
On GitHub Actions the
jesttest run doesn't exit because it is being kept alive.The reason is that we create a
CloudWatchLogMonitorin one test, which keeps on scheduling a timer withsetTimeout()which keeps the Node process alive. I'm not sure why this doesn't manifest on the CodeBuild build or a Mac laptop run, but it can be somewhat reproduced by runningnpx jest --detectOpenHandles, which does make the test hang on Mac as well.I considered
unref()fing the timer, but it seems a smaller and therefore safer change to take theCloudWatchLogMonitorand deactivate it.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license