Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: add deduplication ratio to 'ipfs dag stat' #9787
feat: add deduplication ratio to 'ipfs dag stat' #9787
Changes from 16 commits
f1604da
ac3e6b0
ce81fd0
6dffb58
946fdd6
63d271b
fbc1c9c
eac10df
27547d3
7f38e7e
76e5f20
b48dcb0
9bbef7f
620f73c
9146d95
ee82e97
6641fb3
8d79bcb
f1840ca
864e91b
b689c10
220856f
f2051db
0df029e
08f837c
538f5bb
336a98c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@Jorropo I finished some of the fixes and I'm gonna start coding the tests. I think that for testing this functionality I'm gonna need a few test files, so that I can work with objects that have multiple blocks. Should I add a test files folder in test/cli ?
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.
For now we keep tests in the
test/cli
folder, idk if we would want to change it, seems a bit overkill, you can put helpers elsewhere.I'm surprised you would need more than one file, A golden test is good enough
.car
file in the repoipfs dag import
it.ipfs dag stat ...
vs a set of known correct values from the car file above (called a golden file).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 tips!
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.
Please use the ` syntax, this is very hard to me.
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.
sorry for that, I'm gonna make this more readable
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.
@Jorropo I'm having a hard time comparing the text output with a string literal in this test, I added a test using the json encoded output so that I can validate the deduplication behaviour. Is there any other way that I could test the text output?
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.
@arthurgavazza json is good.
For the text one: the point of golden tests is that the code of the test is very dumb.
You create a dataset ahead of time that show all the behaviour you want (here it would be a
.car
fixture) and then you==
ordiff
the output with some hardcoded values you create at the same time you make your fixture.