Skip to content
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

Make test_launch_ipfs_daemon and pollEndpoint more reliable #8311

Closed
3 tasks done
schomatis opened this issue Jul 28, 2021 · 2 comments
Closed
3 tasks done

Make test_launch_ipfs_daemon and pollEndpoint more reliable #8311

schomatis opened this issue Jul 28, 2021 · 2 comments
Assignees
Labels
topic/technical debt Topic technical debt

Comments

@schomatis
Copy link
Contributor

schomatis commented Jul 28, 2021

Checklist

Description

Spawned from #8131. In that test (it is likely that) the daemon was not running when it was supposed to and we caught it by chance just because there was a slight change in the behavior of a specific command that made the test fail. (Not sure if in that case the daemon failed/crashed at some point or didn't start altogether.)

We should review the pollEndpoint command and add some tests if possible. Additionally it would be useful to leave some check (pollEndpoint or other) running in the background during sharness tests and periodically check that the daemon is running.


Somewhat related to this (but should have its own issue if valuable): As an additional measure the command should be able to tell when it is intending to work through the daemon or not.

Right now we have the NoRemote/NoLocal flags at the command level (in the go-ipfs-cmds/command.go library) but we might want more granularity than that. For example in the above test the --flush=false option in ipfs files write should turn that command into a NoLocal one (to make sure we don't close and flush the local MFS root in the IPFS instance).

Maybe this requirement is too specific to this command alone and in that case just doing an independent check inside the command itself to make sure the daemon is running before executing might be enough.

@schomatis schomatis added the kind/enhancement A net-new feature or improvement to an existing feature label Jul 28, 2021
@schomatis schomatis self-assigned this Jul 28, 2021
@schomatis schomatis added topic/technical debt Topic technical debt and removed kind/enhancement A net-new feature or improvement to an existing feature labels Jul 28, 2021
@schomatis schomatis changed the title Make test_launch_ipfs_daemon and pollEndpoint are more reliable Make test_launch_ipfs_daemon and pollEndpoint more reliable Jul 28, 2021
@schomatis
Copy link
Contributor Author

Some loose working notes to be incorporated later:

  • How do we know the go-ipfs daemon is running at the go-ipfs-cmds level? We check for the api file and dial that (we have access to the repo).

  • In the sharness tests we parse the daemon output that we just instantiated and get the address from there (we have direct access to the daemon). We still wait for the api file to be created before dialing but we don't use its content.

@schomatis
Copy link
Contributor Author

Closing this in favor of more specific actionable items like:

From my review of the sharness test it seems that adding some concurrent check for the daemon to be up in parallel with the running tests in bash is a very involved task and it doesn't seem to have high value at the moment (this should be revisited if we hit more flaky tests like #8131).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic/technical debt Topic technical debt
Projects
None yet
Development

No branches or pull requests

1 participant