Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

test: sharness - add t0040-add-and-cat.sh failing tests #522

Closed
wants to merge 3 commits into from

Conversation

chriscool
Copy link
Contributor

This fails at the second test with:

/usr/bin/nodejs ../../bin/ipfs bootstrap rm <peer>

Options:
  --help  Show help                                                    [boolean]

Not enough non-option arguments: got 0, need at least 1

This is because js-ipfs does not implement ipfs bootstrap rm --all.

@Kubuxu Kubuxu added the status/in-progress In progress label Oct 16, 2016
@chriscool
Copy link
Contributor Author

ipfs bootstrap rm --all is used in a lot of sharness tests, so it is important to support.

@chriscool
Copy link
Contributor Author

This is because it is used by test_init_ipfs() which is in test-lib.sh and most scripts use this function to initialize an ipfs repo.

@daviddias daviddias mentioned this pull request Nov 12, 2016
@daviddias daviddias added js-ipfs-ready and removed status/in-progress In progress labels Nov 21, 2016
@daviddias daviddias added status/deferred Conscious decision to pause or backlog and removed js-ipfs-ready labels Dec 5, 2016
@daviddias
Copy link
Member

@chriscool great news, the bootstrap command was completed -- #669 --, we can now move forward with this.

Can you rebase and try these again?

@daviddias daviddias added status/ready Ready to be worked and removed status/deferred Conscious decision to pause or backlog labels Dec 18, 2016
@daviddias daviddias added status/deferred Conscious decision to pause or backlog and removed status/ready Ready to be worked labels Jan 29, 2017
@daviddias daviddias added exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue labels Feb 13, 2017
@daviddias
Copy link
Member

@chriscool what's missing here today?

@daviddias
Copy link
Member

@chriscool ?

@chriscool
Copy link
Contributor Author

chriscool commented Jul 8, 2017

Hi @diasdavid !
Sorry about the long delay in this.
It is now failing with:

expecting success: 
                IPFS_PID=$! &&
                pollEndpoint -ep=/version -host=$API_MADDR -v -tout=1s -tries=60 2>poll_apierr > poll_apiout ||                                                                                           
                test_fsh cat actual_daemon || test_fsh cat daemon_err || test_fsh cat poll_apierr || test_fsh cat poll_apiout
> cat actual_daemon
Initializing daemon...
Starting at /home/utilisateur/git/tests_js_ipfs/js-ipfs/test/sharness/trash directory.t0040-add-and-cat.sh/.ipfs
Swarm listening on /ip4/127.0.0.1/tcp/36643
Swarm listening on /ip4/192.168.1.63/tcp/36643
API is listening on: http://127.0.0.1:35231
Gateway (readonly) is listening on: http://127.0.0.1:43157
Daemon is ready
> cat daemon_err
> cat poll_apierr
./t0040-add-and-cat.sh: 3: eval: pollEndpoint: not found

So it is looking for pollEndpoint which is a go helper program. See:

https://github.com/ipfs/go-ipfs/tree/master/thirdparty/pollEndpoint
https://github.com/ipfs/go-ipfs/search?utf8=%E2%9C%93&q=pollEndpoint&type=

I don't know if it is ok to add pollEndpoint as is or if it should be rewritten in js or maybe replaced with something else.

@chriscool
Copy link
Contributor Author

I just pushed a branch rebased on top of master if someone wants to take a look.

@daviddias
Copy link
Member

No prob @chriscool, it seems that pollEndpoint is just checking if the daemon is still running by checking its version -- https://github.com/ipfs/go-ipfs/search?utf8=%E2%9C%93&q=pollEndpoint&type= --. Can we skip that check? If not, you should be able to use go pollEndpoint, the http api is implemented in the same way.

@chriscool
Copy link
Contributor Author

@diasdavid pollEndpoint contains:

	logging "gx/ipfs/QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52/go-log"
	manet "gx/ipfs/QmX3U3YXCQ6UYBxq2LVWF8dARS1hPUTEYLrSx654Qyxyw6/go-multiaddr-net"
	ma "gx/ipfs/QmXY77cVe7rVRQXZZQRioukUM7aRW3BTcAgJe12MCtb3Ji/go-multiaddr"

So it requires a number of go packages and I am not sure that it would be a good idea to require more go software.

pollEndpoint is used by test_launch_ipfs_daemon() to check that the IPFS daemon is ready:

https://github.com/ipfs/go-ipfs/blob/fbdb896105e3c59068e7889db0fabce90b90acf6/test/sharness/lib/test-lib.sh#L203-L226

I think this check is quite important, so I don't think it would be a good idea to skip it.

One solution could be to put pollEndpoint into its own separate repo and just require it to be installed.

For example go-random is also required for this test to work, but it has its own separate repo, so it's less intrusive.

@chriscool
Copy link
Contributor Author

I put pollEndpoint into a new repo:

https://github.com/ipfs/go-poll-endpoint

@daviddias
Copy link
Member

@chriscool got your ping. Seems that you already extracted go-pollendpoint, you will have to compile it for different archs and make sure the right one gets used depending on where it is run.

Do we just want to check if the daemon is on? Do we need a custom program? Why not just netcat or curl to see if the socket is still open?

@chriscool
Copy link
Contributor Author

@diasdavid there are a number of go programs that are used by the test suite, like go-random, go-sleep, etc. See: https://github.com/ipfs/go-ipfs/blob/master/test/sharness/Rules.mk

So it's not just about go-poll-endpoint. And it looks like the docker image used for the Travis tests doesn't even have go installed.
Is there a reason why it is not the same image as for go-ipfs tests? Should we create a new image with go installed?
If we have to install go before running the tests, it will take a lot of time and bandwidth each time we run a test which is not good.

@daviddias daviddias added status/ready Ready to be worked status/in-progress In progress P2 Medium: Good to have, but can wait until someone steps up and removed status/deferred Conscious decision to pause or backlog status/ready Ready to be worked labels Oct 17, 2017
@daviddias
Copy link
Member

@victorbjelkholm is planning to extract sharness onto its own repo so that it can be used against multiple IPFS implementations. You two should sync up :)

@daviddias daviddias added exp/expert Having worked on the specific codebase is important status/ready Ready to be worked and removed exp/novice Someone with a little familiarity can pick up help wanted Seeking public contribution on this issue status/in-progress In progress labels Jan 25, 2018
@chriscool
Copy link
Contributor Author

@diasdavid yeah I already talked to @victorbjelkholm but I will sync up again.

@chriscool
Copy link
Contributor Author

See also: ipfs-inactive/dev-team-enablement#12

@daviddias daviddias changed the title sharness: add t0040-add-and-cat.sh failing tests test sharness - add t0040-add-and-cat.sh failing tests May 29, 2018
@daviddias daviddias changed the title test sharness - add t0040-add-and-cat.sh failing tests test: sharness - add t0040-add-and-cat.sh failing tests May 30, 2018
@alanshaw
Copy link
Member

@chriscool I'm guessing this can be closed due to the work you'll be doing on using the sharness repo with JS IPFS?

@chriscool
Copy link
Contributor Author

@alanshaw yeah, you can close this.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/expert Having worked on the specific codebase is important P2 Medium: Good to have, but can wait until someone steps up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants