-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix AboutPage test console error output #1031
Conversation
@@ -24,6 +24,8 @@ describe('AboutPage component', () => { | |||
it('should render the about page with default values if api get request fails', async () => { | |||
const stateValues = { flavor: 'N/A', subscriptions: 0, version: 'v0.0.0' }; | |||
const errorMessage = { messages: "Get request '/api/about' failed" }; | |||
jest.spyOn(console, 'error').mockImplementation(() => null); |
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.
Maybe we can have an helper called silenceErrorMessages
or something, but as I write this I have to say that I'm not a big fan of silencing error messages
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.
Well, in this is case is a known error, so it is fine i think
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.
@arbulu89 if I'm not wrong this is the second time we set a similar spy, do we wanna have an helper?
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.
Personally I wouldn't. It is simply a one liner. But up to @EMaksy
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.
Then we leave it this way. LGTM
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.
OK fine by me, maybe when this problem reappears in the future we can discuss adding a helper!
@@ -24,6 +24,8 @@ describe('AboutPage component', () => { | |||
it('should render the about page with default values if api get request fails', async () => { | |||
const stateValues = { flavor: 'N/A', subscriptions: 0, version: 'v0.0.0' }; | |||
const errorMessage = { messages: "Get request '/api/about' failed" }; | |||
jest.spyOn(console, 'error').mockImplementation(() => null); |
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.
Well, in this is case is a known error, so it is fine i think
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.
LGTM
Description
Every time we run the AboutPage test, there is an annoying console log error message, even when the test runs perfect.
The pr will remove the message, so the console log of the CI is not misleading.