Skip to content
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

doctor: fix errors when there is no config #1397

Merged
merged 1 commit into from
Jan 26, 2022
Merged

doctor: fix errors when there is no config #1397

merged 1 commit into from
Jan 26, 2022

Conversation

marcelometal
Copy link
Member

No description provided.

@heynemann
Copy link
Member

Hey good catch! Do you mind adding a test for when there's no config? I missed that one. There's a test file that tests doctor outputs.

Copy link
Member

@heynemann heynemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned above, pending only adding a unit test.

@heynemann
Copy link
Member

Gotta fix another bug :( Sorry. There's a sys.exit if no errors found and that sys.exit should also respect the exit flag.

@heynemann
Copy link
Member

Just need to update the snapshot and you should be good to go :)

@heynemann
Copy link
Member

You force-pushed over my change :(

@heynemann
Copy link
Member

Just remove the sys.exit from the print message method.

@marcelometal
Copy link
Member Author

marcelometal commented Jan 26, 2022

@heynemann ,

You force-pushed over my change :(

I'm sorry!

Just need to update the snapshot and you should be good to go :)

Did you add --snapshot-update in make test?

@heynemann
Copy link
Member

Nope and you shouldn't, otherwise tests will never break. The idea of snapshots is that you look at the snapshot and it is what you expect, then it never changes (unless the functionality changes). This ensures the result is always the same.

@heynemann heynemann merged commit 63c9f9f into master Jan 26, 2022
@heynemann heynemann deleted the fix-doctor branch January 26, 2022 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants