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

console debug log level #190

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

console debug log level #190

wants to merge 4 commits into from

Conversation

KaelynJefferson
Copy link
Collaborator

Description: This PR adds a debug log level option to the configuration settings menu to allow a user to toggle the debug level messages that appear in the console.

Testing Instructions: Run jest unit tests and view within browser.

Related Issue: n/a

Copy link
Collaborator

@mint-thompson mint-thompson left a comment

Choose a reason for hiding this comment

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

This looks good! I tried it out locally, and sure enough, I see the expected logging messages when I change the option. I have one small question about the tests.

expect(onGoFSHClick).toHaveBeenCalledTimes(2);
expect(onGoFSHClick).toHaveBeenCalledWith('', true); // Loading
expect(onGoFSHClick).toHaveBeenCalledWith(simpleFsh, false);
});
});

it('calls GoFSH with the logger level debug if the configuration checkbox is checked', async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should there also be a test for running SUSHI with debug logging enabled?

Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

This is a nice feature! I've reviewed the code and tested it manually. Debug logging works for both SUSHI and GoFSH. Thanks, @KaelynJefferson!

Copy link
Collaborator

@mint-thompson mint-thompson left a comment

Choose a reason for hiding this comment

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

Thanks for adding that test!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants