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

add test for bottom view binary tree #147

Merged
merged 3 commits into from
Oct 26, 2019
Merged

Conversation

christian-fei
Copy link
Contributor

this should help with #146

please let me know what you think

Copy link
Member

@TheSTL TheSTL left a comment

Choose a reason for hiding this comment

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

hi @christian-fei can you cover following test cases where Btree is null?

@christian-fei
Copy link
Contributor Author

@TheSTL updated the tests, thanks

btree = new BinaryTree([1, 2, 3, 4, 5, 6]);
});

it('Determines the bottom view of a binary tree', () => {
Copy link
Member

Choose a reason for hiding this comment

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

'Determines the bottom view of a binary tree' -> Should determines the bottom view of a binary tree
It's all about Behavior Driven Testing, hence it starts with or contains should

it('Determines the bottom view of a binary tree', () => {
expect(bottomView(btree)).toEqual([6, 2, 3, 4]);
});
it('handles null binary tree', () => {
Copy link
Member

Choose a reason for hiding this comment

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

'handles null binary tree' -> Should handles null binary tree

Copy link
Member

@TheSTL TheSTL left a comment

Choose a reason for hiding this comment

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

Learn today how to write unit test title.
Sorry for pointing it out now. @christian-fei

Copy link
Member

@TheSTL TheSTL left a comment

Choose a reason for hiding this comment

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

Thanks @christian-fei for PR 💯

@TheSTL TheSTL merged commit 29b1650 into knaxus:master Oct 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants