Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add unit tests to Max Product of 3 numbers problem #26

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

Merged
ashokdey merged 2 commits into knaxus:master from LeoSL:add-test-to-max-product-of-3-numbers
Oct 4, 2019

Conversation

Copy link
Contributor

@LeoSL LeoSL commented Oct 4, 2019

🐻 What's inside?

  • Add unit tests to Max Product of 3 numbers problem
  • Export function to be used by test file

🚨Issue found

Since the problem specifies the product of exactly 3 numbers I was about to write the following test...

it("returns an error if there are less than 3 numbers", () => {
 expect(() => {
 maxProductof3Numbers([-10, -1]);
 }).toThrowError();
});

But this case is not covered by the implementation. Should I implement this test scenario?

ashokdey and TheSTL reacted with thumbs up emoji
Copy link
Member

ashokdey commented Oct 4, 2019

Thanks for the PR @LeoSL, really good to see the edge case you pointed out. @TheSTL will carry on with this PR.

All the best 👍

Copy link
Member

TheSTL commented Oct 4, 2019

Thanks @LeoSL .
I implement this scenario in this Pr #28 .
Now you can implement test scenario where array length is less than 3.

Copy link
Member

ashokdey commented Oct 4, 2019

🐻 What's inside?

  • Add unit tests to Max Product of 3 numbers problem
  • Export function to be used by test file

🚨Issue found

Since the problem specifies the product of exactly 3 numbers I was about to write the following test...

it("returns an error if there are less than 3 numbers", () => {
 expect(() => {
 maxProductof3Numbers([-10, -1]);
 }).toThrowError();
});

But this case is not covered by the implementation. Should I implement this test scenario?

Hello @LeoSL, since @TheSTL has handled the edge case, you can move forward to implement the test case.

Copy link
Contributor Author

LeoSL commented Oct 4, 2019

Hey @ashokdey @TheSTL , thank you for the quick response and action.

I went ahead and committed the test case on this PR. Let me know if there are other details to fix.

TheSTL reacted with hooray emoji

@ashokdey ashokdey merged commit c78f6ed into knaxus:master Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashokdey ashokdey ashokdey approved these changes

@TheSTL TheSTL TheSTL approved these changes

Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /