-
Notifications
You must be signed in to change notification settings - Fork 20
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
Boavizta test coverage #69
Conversation
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
@@ -100,6 +129,36 @@ describe('lib/boavizta: ', () => { | |||
expect(await output.execute([])).toEqual([]); | |||
}); | |||
|
|||
it('throws an error when the metric type is `gpu-util`.', async () => { |
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.
title says it's throws an error, but the implementation says it has result
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.
changed
}, | ||
]); | ||
} catch (error) { | ||
expect(error).toBeInstanceOf(UnsupportedValueError); |
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.
please also assert the error message if possible
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.
done
@@ -64,6 +64,35 @@ describe('lib/boavizta: ', () => { | |||
]); | |||
}); | |||
|
|||
it('returns a result when the impacts from the API is undefined.', async () => { |
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.
why we are returning valid result if the API response is undefined?
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.
updated description. We set it to 0 in case of there isn't data from the API
Types of changes
A description of the changes proposed in the Pull Request