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
Check it out if I did the right thing. #1938
base: George-prog275
Are you sure you want to change the base?
Conversation
Implemented the _check function on calculator.js file.
Thank you for the submission, @George-prog275! I'll review your code shortly, hang tight. |
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.
This is a great attempt, @George-prog275!
I would like to request a few changes before merging your work. Please review my comments below and make the appropriate changes to your code.
After you update your code locally, follow the instructions to save your changes locally and push your changes to your fork.
When you push your changes to your fork, I'll come back for another review.
There are 1 style guide violations in your contribution. I've marked them with inline comments for your convenience.
Please revisit your code and follow the style guide best practices.
Hint: You might be able to fix some issues automatically by running npm run lint -- --fix
There are 8 tests failing. Please revisit your code and make the failing tests pass.
❌ _check should throw a TypeError if arguments are not numbers
-actual
+expected
-
+function TypeError() { [native code] }
AssertionError: expected [Function] to throw TypeError
at Context.<anonymous> (src/calculator.test.js:17:54)
at processImmediate (node:internal/timers:464:21)
❌ _check should be called once in "subtract"
AssertionError: expected _check to have been called with arguments 44, 2
�[32m44�[0m
�[32m2�[0m
at Context.<anonymous> (src/calculator.test.js:30:44)
at processImmediate (node:internal/timers:464:21)
❌ _check should be called once in "multiply"
AssertionError: expected _check to have been called with arguments 6, 7
�[32m6�[0m
�[32m7�[0m
at Context.<anonymous> (src/calculator.test.js:36:44)
at processImmediate (node:internal/timers:464:21)
❌ _check should be called once in "divide"
AssertionError: expected _check to have been called with arguments 84, 2
�[32m84�[0m
�[32m2�[0m
at Context.<anonymous> (src/calculator.test.js:42:44)
at processImmediate (node:internal/timers:464:21)
❌ add should throw a TypeError if arguments are not numbers
-actual
+expected
-
+function TypeError() { [native code] }
AssertionError: expected [Function] to throw TypeError
at Context.<anonymous> (src/calculator.test.js:48:51)
at processImmediate (node:internal/timers:464:21)
❌ subtract should throw a TypeError if arguments are not numbers
-actual
+expected
-
+function TypeError() { [native code] }
AssertionError: expected [Function] to throw TypeError
at Context.<anonymous> (src/calculator.test.js:71:56)
at processImmediate (node:internal/timers:464:21)
❌ multiply should throw a TypeError if arguments are not numbers
-actual
+expected
-
+function TypeError() { [native code] }
AssertionError: expected [Function] to throw TypeError
at Context.<anonymous> (src/calculator.test.js:94:56)
at processImmediate (node:internal/timers:464:21)
❌ divide should throw a TypeError if arguments are not numbers
-actual
+expected
-
+function TypeError() { [native code] }
AssertionError: expected [Function] to throw TypeError
at Context.<anonymous> (src/calculator.test.js:117:54)
at processImmediate (node:internal/timers:464:21)
@@ -1,7 +1,10 @@ | |||
/* eslint-disable no-unused-expressions */ | |||
const calculator = require('./calculator'); | |||
|
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.
More than 2 blank lines not allowed.
(learn more)
@@ -11,9 +14,6 @@ describe.skip('_check', () => { | |||
}); | |||
|
|||
it('should throw a TypeError if arguments are not numbers', () => { | |||
expect(() => calculator._check(40, '2')).to.throw(TypeError); | |||
expect(() => calculator._check(40, [])).to.throw(TypeError); | |||
expect(() => calculator._check(40, {})).to.throw(TypeError); | |||
expect(() => calculator._check('40', 2)).to.throw(TypeError); |
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.
This line is responsible for 1 failing tests. Run npm test
and see if you can figure out why.
❌ _check should throw a TypeError if arguments are not numbers
Implemented the _check function on calculator.js file.
Delete me and write your Pull Request message here!