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

rpc: add uptime #77

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

rpc: add uptime #77

wants to merge 2 commits into from

Conversation

slashrsm
Copy link
Contributor

@slashrsm slashrsm commented Aug 14, 2018

Fixes #51.

I am playing with bcash and thought that this issue would be a nice intro into the codebase.

Depends on the related pull bcoin-org/bclient#14 to pass the test.

package.json Outdated
@@ -62,6 +62,7 @@
"istanbul": "^1.1.0-alpha.1",
"jsdoc": "^3.5.5",
"mocha": "^5.2.0",
"mockdate": "^2.0.2",
Copy link
Member

Choose a reason for hiding this comment

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

I like how mockdate has no dependencies itself, but adding dependencies to this project will make it very difficult to have your PR accepted. Would it be easy to write the test without adding the dependency?

@tynes
Copy link
Member

tynes commented Aug 14, 2018

Also see here:

http://bcoin.io/api-docs/index.html#get-server-info

curl $url | jq .time.uptime

@slashrsm
Copy link
Contributor Author

Thank you for the feedback! I understand the concern about adding an additional dependency. Uptime seems to always be 0 in the test environment. We could simply assert for that to make sure that something sensible is returned or mock the date on our own (it looks failry simple to do so...).

Uptime isn't covered by the test that covers the server info request. Date mock could also benefit that and other time-related stuff I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants