Skip to content
This repository was archived by the owner on Mar 10, 2020. It is now read-only.

test: add human readable repo stats test #554

Closed
wants to merge 2 commits into from

Conversation

PedroMiguelSS
Copy link
Contributor

Add human-readable repo stats test.

@PedroMiguelSS
Copy link
Contributor Author

Can I get a second review here, please? @alanshaw @hugomrdias

expect(res).to.have.a.property('storageMax')
expect(res.numObjects).to.be.a('number')
expect(res.repoSize).to.match(/\s.?B$/gm)
expect(res.storageMax).to.match(/\s.?B$/gm)
Copy link
Contributor

@alanshaw alanshaw Nov 18, 2019

Choose a reason for hiding this comment

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

/\s.?B$/gm - hmm that doesn't look right to me.

Screenshot 2019-11-18 at 15 32 19

Are the gm modifiers needed? Does the test pass? If not, please ensure the test passes locally before requesting a review. If you need any help linking and running only this test then ping me on IRC.

Copy link
Contributor Author

@PedroMiguelSS PedroMiguelSS Nov 19, 2019

Choose a reason for hiding this comment

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

Yap, sorry, the modifiers are not needed @alanshaw. Furthermore, I'll update the expression, I'll make it more specific.
I wouldn't have asked for a second review if the test was not passing locally. You can see it passing here:
image

This is the res object

{
   ...
   repoSize: '30.1 KB'
   storageMax: '9.01 PB'
}

My regex was matching:
image
and
image

What do you think about this regex? Does this one seem better? It would match the whole string instead of just the units.
/^[\d]+(\.[\d]+)?\s[KMGTP]?B$/

Considering the res object (mentioned above), this new regex will match:
image
and
image

@alanshaw
Copy link
Contributor

This can be closed now right @PedroMiguelSS?

@PedroMiguelSS PedroMiguelSS deleted the test/add-human-option-test-repo-stat branch November 25, 2019 12:08
@PedroMiguelSS
Copy link
Contributor Author

Closed in favor of ipfs/js-ipfs#2630.

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

Successfully merging this pull request may close these issues.

3 participants