-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[NBS 1.0] deprecate legacy logger helpers #24804
[NBS 1.0] deprecate legacy logger helpers #24804
Conversation
Signed-off-by: Camila Belo <camilaibs@gmail.com>
Signed-off-by: Camila Belo <camilaibs@gmail.com>
Changed Packages
|
/** @internal */ | ||
function createLoggerMock() { | ||
return { | ||
child: jest.fn().mockImplementation(createLoggerMock), |
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.
Note
For some tests, this was failing because the child function returned undefined
previously. There is a fragility with this solution, if the resetAllMocks
is used, the implementation mock will be removed and the tests calling child
will fail again. Any suggestions to prevent this from happening?
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.
Hmm, if you use jest.fn(createLoggerMock)
here instead, does that have an effect?
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.
It has no effect, if reset is called, child becomes just a noop function
@@ -79,7 +79,7 @@ describe('AwsS3EntityProvider', () => { | |||
}); | |||
|
|||
afterEach(() => { | |||
jest.resetAllMocks(); | |||
jest.clearAllMocks(); |
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.
Note
As commented before, I'm replacing jest.resetAllMocks()
with jest.clearAllMocks( )
to avoid resetting the child
method mock implementation.
Signed-off-by: Camila Belo <camilaibs@gmail.com>
Signed-off-by: Camila Belo <camilaibs@gmail.com>
89380c8
to
6a576dc
Compare
packages/backend-app-api/src/services/implementations/cache/cacheServiceFactory.ts
Show resolved
Hide resolved
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.
👌 🧹 ✨
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.
Looks good for DevTools, thanks @camilaibs 🚀
2c39e74
to
d025975
Compare
Signed-off-by: Camila Belo <camilaibs@gmail.com>
d025975
to
e49d0fd
Compare
…mmons Signed-off-by: Camila Belo <camilaibs@gmail.com>
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.
🎉
Thank you for contributing to Backstage! The changes in this pull request will be part of the |
Uffizzi Cluster |
Hey, I just made a Pull Request!
Refs: #24803
It is part of the effort to deprecate the legacy backend system and this pull request deprecates the logger helpers.
✔️ Checklist
Signed-off-by
line in the message. (more info)