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
Closed

test: add human readable repo stats test #554

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6aca09
test: add human readable repo stats test
Nov 8, 2019
c432f7d
chore: code review changes
Nov 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/repo/stat.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,21 @@ module.exports = (createCommon, options) => {
expectIsRepo(null, res)
})
})

it('should get human readable repo stats', async () => {
const res = await ipfs.repo.stat({ human: true })

expect(res).to.exist()
expect(res).to.have.a.property('numObjects')
expect(res).to.have.a.property('repoSize')
expect(res).to.have.a.property('repoPath')
expect(res).to.have.a.property('version')
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 •
edited
Loading

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 •
edited
Loading

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

expect(res.repoPath).to.be.a('string')
expect(res.version).to.be.a('string')
})
})
}