Skip to content

Test ipfs2 with sharness#343

Closed
chriscool wants to merge 4 commits intoipfs:masterfrom
chriscool:test_ipfs2
Closed

Test ipfs2 with sharness#343
chriscool wants to merge 4 commits intoipfs:masterfrom
chriscool:test_ipfs2

Conversation

@chriscool
Copy link
Copy Markdown
Contributor

@maybebtc @jbenet

The goal of this branch is to help make the sharness tests pass with ipfs2.
It can be merged into a test branch in jbenet/go-ipfs, so that we can see what happens on travis.

The first patch in this PR is 3368d29. It was made by @maybebtc to make the sharness tests use ipfs2.

The second patch make the output of some tests match the output from some ipfs2 commands. It's possible we should instead change the output from the ipfs2 commands to match the tests, or that we should change both the tests and the output from ipfs2.

@jbenet
Copy link
Copy Markdown
Member

jbenet commented Nov 15, 2014

Great!

The second patch make the output of some tests match the output from some ipfs2 commands. It's possible we should instead change the output from the ipfs2 commands to match the tests, or that we should change both the tests and the output from ipfs2.

I see two changes:

  1. the add cmd changed to not show the absolute dir. ipfs2 has the desired behavior (i.e. matches the input paths)
  2. the block cmd changed its output. ipfs1 has the desired behavior

@chriscool
Copy link
Copy Markdown
Contributor Author

Yeah, there are the two changes you mention.
Now, what I can do is split the patch into 2 patches. This way, when the block cmd will have the desired behavior again, we can just drop the related patch.

I am also looking at the ways to adapt the ipfs mount tests.
It looks like with ipfs2, "ipfs daemon" has to be launched before "ipfs mount", and that "ipfs mount" does not accept an argument any more.

@jbenet
Copy link
Copy Markdown
Member

jbenet commented Nov 15, 2014

Just pushed the desired change for ipfs block: 8a284c9

It looks like with ipfs2, "ipfs daemon" has to be launched before "ipfs mount"

Yes, this is correct. It's a bit annoying but the exact UX is hard to get right. We opted to try out cleanly separating daemon commands as requiring the daemon to be run explicitly. Not yet sure this is TRTTD, but it's an experiment. what are your thoughts?

"ipfs mount" does not accept an argument any more.

Ah! This is not good. it should. Tried it and see this:

% ipfs2 mount -f ipfs
Error: Expected 0 arguments, got 1

USAGE:

    ipfs mount - Mounts IPFS to the filesystem (read-only)
...

Yet, the options look correct, and even show up in the help:

% ipfs2 mount --help

    ipfs mount - Mounts IPFS to the filesystem (read-only)

OPTIONS:

    -f string - The path where IPFS should be mounted
    -n string - The path where IPNS should be mounted

@mappum any ideas?

@chriscool
Copy link
Copy Markdown
Contributor Author

@jbenet

I force pushed the branch and now the 2 changes are now in 2 different patches. The patch related to cmd block does not take into account your change to the output, but it will be easy to do.

About cmds mount and daemon, maybe there should be a short option available in mount that starts the daemon like -s (for start) or -d (for daemon), but we can add that later.

@chriscool
Copy link
Copy Markdown
Contributor Author

About "ipfs help", we are using egrep "^Usage: +ipfs" to see if its output looks good.
But with ipfs2, the output looks like:

USAGE:

    ipfs - Global P2P Merkle-DAG filesystem

    Basic commands:

        init          Initialize ipfs local configurationx
        add <path>    Add an object to ipfs
        cat <ref>     Show ipfs object data
        ls <ref>      List links from an object

    Tool commands:
...

I would expect at least something like "Usage: ipfs [options] [arguments]" at the top, otherwise it's not very clear if ipfs will launch its own shell (like when you launch "python") or if it expects commands as arguments after "ipfs".

@chriscool
Copy link
Copy Markdown
Contributor Author

Also with ipfs2, the "ipfs help" output goes to stderr instead of stdout previously.

@jbenet
Copy link
Copy Markdown
Member

jbenet commented Nov 15, 2014

About cmds mount and daemon, maybe there should be a short option available in mount that starts the daemon like -s (for start) or -d (for daemon), but we can add that later.

I like that idea -- #346

I would expect at least something like "Usage: ipfs [options] [arguments]" at the top, otherwise it's not very clear

Hm, this is a bit non trivial, given how we setup autogeneration. How's the following (from a4488ca):

USAGE:

    ipfs - global p2p merkle-dag filesystem

SYNOPSIS

    ipfs [<flags>] <command> [<arg>] ...

    Basic commands:

        init          Initialize ipfs local configurationx
        add <path>    Add an object to ipfs
        cat <ref>     Show ipfs object data
        ls <ref>      List links from an object

    Tool commands:

        config        Manage configuration
        update        Download and apply go-ipfs updates
        version       Show ipfs version information
        commands      List all available commands

    Advanced Commands:

        daemon        Start a long-running daemon process
        mount         Mount an ipfs read-only mountpoint
        serve         Serve an interface to ipfs
        diag          Print diagnostics

    Plumbing commands:

        block         Interact with raw blocks in the datastore
        object        Interact with raw dag nodes

    Use 'ipfs <command> --help' to learn more about each command.

Also with ipfs2, the "ipfs help" output goes to stderr instead of stdout previously.

Fixed in 3df1513.

@chriscool
Copy link
Copy Markdown
Contributor Author

@jbenet yeah, it is much better now, thanks!

Brian Tiger Chow and others added 4 commits November 15, 2014 20:06
@jbenet @chriscool

(not to be merged into master) This is a hack to run sharness tests on
th ipfs2 binary. Instead of compiling cmd/ipfs, it compiles cmd/ipfs2
and copies this into test/bin/ipfs.

I thought this would be enough to pass the `basic-commands` test, but
it's not.

Although the output is fairly similar, the `ipfs version` test fails.

```
test (feat/test2) λ. diff version1 version2
1c1
< ipfs version 0.1.7
---
> ipfs version 0.1.5
```

I'm not very experienced with `sh` scripting, so perhaps I'm missing a
key ingredient or maybe misunderstanding the the tests are meant to
work.

Would like to get input on this.

Thanks, @maybebtc
The ouput from "ipfs add" changed in ipfs2.
With the changes in this commit, this output change doesn't
prevent the test to pass.

With TEST_NO_FUSE=1 test t0040 now passes on my Linux machine.

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
The ouput from "ipfs block" changed in ipfs2.
With the change in this commit, this output change doesn't
prevent the test to pass.

Test t0050 now passes on my Linux machine.

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
The ouput from "ipfs help" changed in ipfs2.
With the change in this commit, this output change doesn't
prevent the test to pass.

Test t0010 now passes on my Linux machine.

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@chriscool
Copy link
Copy Markdown
Contributor Author

PR #347 should be discussed instead of this one now as it contains c2b021a which is needed for t0010.

@btc btc closed this Nov 16, 2014
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