-
Notifications
You must be signed in to change notification settings - Fork 63
Add unit tests #62
base: main
Are you sure you want to change the base?
Add unit tests #62
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!
.gitignore
Outdated
npm-debug.log* | ||
yarn-debug.log* | ||
yarn-error.log* | ||
.idea |
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.
.idea | |
.idea | |
We should add documentation on our standards for formatting, but I gently suggest we terminate all files with a new line per unix standards
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.
Thanks for the suggestion but I am going to leave it as it is. The deadline is approaching and we need this merged ASAP.
I left a TODO comment so we can handle this issue on a future PR :)
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
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 art.
LGTM |
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.
Looks like the tests are currently failing, can you add some more tethers to the price.
|
||
test('it should be able to sell high', () => { | ||
const finalPrice = getBitcoinPriceToday() | ||
expect(finalPrice).toBeGreaterThan(initialPrice); |
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.
expect(finalPrice).toBeGreaterThan(initialPrice); | |
expect(finalPrice).toBeGreaterThan(initialPrice) |
Consistent terminating characters.
yarn-error.log* | ||
.idea | ||
|
||
TODO: terminate files with new line per unix standard |
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.
The sensible choice of terms here would be end of line (EOL) given it is intended here to represent the end of a line, not the beginning of an always empty line.
TODO: terminate files with new line per unix standard | |
TODO: terminate files with end of line per unix standard |
No description provided.