-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add test for ipfs mount #248
Conversation
License: MIT Signed-off-by: Christian Couder <[email protected]>
Is there a cleanup that deletes the .go-ipfs dir that these tests create? |
Yeah, test scripts are executed in a directory named "trash directory.t00X0-name.sh" which is deleted if the test script succeeds. For example t0030-mount.sh is executed in: trash directory.t0030-mount.sh/ You can add an "exit 1" into t0030-mount.sh and you will see. By the way I will add a rule to ignore these test directories into the ".gitignore". |
License: MIT Signed-off-by: Christian Couder <[email protected]>
License: MIT Signed-off-by: Christian Couder <[email protected]>
Youre awesome. These tests are awesome. everything LGTM |
Thanks! |
This adds test_launch_ipfs_mount() and test_kill_ipfs_mount() to avoid duplicating tests to launch "ipfs mount" and to kill it. License: MIT Signed-off-by: Christian Couder <[email protected]>
License: MIT Signed-off-by: Christian Couder <[email protected]>
@jbenet could ou have a look at this PR? |
@@ -1 +1,2 @@ | |||
test-results/ | |||
trash directory.*.sh/ |
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.
could we name this trash-directory...
? (replace/ /-
)
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 space in the directory name was added on purpose during Git development to force people to properly quote arguments which is often a problem in shell scripts.
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.
Ahh, nice! Indeed 👍 😄
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.
Yeah, and if we want to support Windows, it is especially important to check that it works with a path that contains spaces as programs are very often installed in "C:\Program Files\stuff".
@chriscool yep! sorry for delay. agreed with @whyrusleeping these tests are great and very much needed!
|
Also:
|
About the test failure, could you use hexdump -C on the expected and actual files to make sure that there is no difference? |
About using a 100MB file with entropy, I can generate one with something like:
Hopefully it is portable enough. |
@jbenet I think it is a bug in ipfs that sometimes "cat ipfs/$HASH" would output not just the content of the $HASH object but also stuff like "Unmounting /Users/jbenet..." |
Ah, yes, sorry. I didn't even stop to consider that! Not sure why it's outputting Unmounting... for |
@chriscool you can use https://github.com/jbenet/go-random -- it should be portable, and uses |
@jbenet I think it is a bug in ipfs or fuse (or somewhere in between) that sometimes "cat ipfs/$HASH" would output stuff (like "Unmounting...") that is not in the object referenced by $HASH. |
@chriscool sadly that will only work on unix based systems... and even though windows hasnt been in our dev scope for a while, we do want to support it eventually. |
@jbenet I think it is a bug in ipfs or fuse (or somewhere in between) that sometimes "cat ipfs/$HASH" would output stuff (like "Unmounting...") that is not in the object referenced by $HASH. |
Re: portability of random file, we can build a tiny, portably go cmd that reads and outputs from
|
@chriscool ah wait, no thats not the problem. That ( Here's the proper hexdumps:
cat seems to be adding extra stuff at the end. (still an error on our side) cc @whyrusleeping weren't we seeing this earlier? random trailing zeroes? |
@chriscool I meant there is a difference in the files ("Unmounting....") sorry if my wording came off as the opposite.
|
@jbenet I think it is a bug in ipfs or fuse (or somewhere in between) that sometimes "cat ipfs/$HASH" would output stuff (like "Unmounting...") that is not in the object referenced by $HASH. |
What I can do is I can use test_expect_failure instead of test_expect_success in the test script, so that we document that this test might fail. Also with a 100MB file I get a different hash everytime I add the same file. It looks like a bug to me. |
Ooops I don't know what happened to this PR but it loosk like some comments have been duplicated, and I thought that some of my comments were lost, so I reposted them. |
No, it's definitely a failure, the test is correct (i.e. leave as is). We should fix this problem in ipfs.
Yep, that's certainly a bug. But-- is your
|
Yes, there's some github bugs happening. see the picture in #248 (comment) and compare to reality. |
About the bug with trailing zeros and "Unmounting...", just to make sure we agree, it is a bug when running "cat ipfs/$HASH" where "ipfs/" is the directory where ipfs is mounted. What you posted doesn't mean a bug when runnning "ipfs cat $HASH", though maybe there is also the same bug when runnning "ipfs cat $HASH" but it is not triggered for some reason. Also you are right, I am not running the latest version from master, because I didn't upgrade yet to go 1.3. I am still running go 1.2.1, but I will upgrade soon. One last point, after thinking about it some more, I am not sure creating the 100MB with pure random stuff is the best thing to do. Maybe using a pseudo random generator is better, because this way we can check that the hash we get back when we "ipfs add" the file is always the same hardcoded value. This way we check that there is no regression in the hash function on a big file. But I agree that as we want to support Windows, it is better if can rely on a simple go program like go-random. |
Hahahaha, you guys had a daylight savings time bug |
I created this PR to have a pseudo random generator: |
jbenet/go-random#1 merged, thanks! |
Great thanks! I have to sleep now, so I will finish this later, hopefully before next week end. |
@chriscool no worries :) 👍 thanks! |
License: MIT Signed-off-by: Christian Couder <[email protected]>
@jbenet please tell me if you need something else to merge this PR |
Hey @chriscool -- this is great! The tests fail for me-- presumably because of the extra zeroes at the ends of things. If the tests are correct (confirm?), let's merge it in and fix those bugs in ipfs. As for travis, I'll add it now. it's just modifying the |
The tests work on my machine. I think they are correct. |
MacOSX is known for having fuse bugs, one of which i havent actually gotten around to fixing yet |
(but i do know the solution... maybe that should be my next priority) |
@chriscool ok great, I'll merge this then |
Fix a race in dial queue
This is one more step related to issue #148 (Whole-tool test).