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

fuse travis-ci #2120

Closed
wants to merge 0 commits into from
Closed

fuse travis-ci #2120

wants to merge 0 commits into from

Conversation

thelinuxkid
Copy link
Contributor

@jbenet jbenet added the backlog label Dec 23, 2015
@thelinuxkid thelinuxkid mentioned this pull request Dec 23, 2015
2 tasks
@ghost ghost added the kind/test Testing work label Dec 23, 2015
if err != nil {
return err
if n.PrivateKey == nil {
if err := n.LoadPrivateKey(); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe instead of having this, we should probably just make LoadPrivateKey not fail if called while the key is already loaded

@thelinuxkid
Copy link
Contributor Author

Yes. I was afraid to break something but that makes sense. Updated and tests are passing locally.

@thelinuxkid thelinuxkid mentioned this pull request Dec 29, 2015
3 tasks
@daviddias daviddias removed the backlog label Jan 2, 2016
@thelinuxkid
Copy link
Contributor Author

Working on a rebase for this

@jbenet
Copy link
Member

jbenet commented Jan 14, 2016

Great thanks!
On Wed, Jan 13, 2016 at 13:19 Andres Buritica notifications@github.com
wrote:

Working on a rebase for this


Reply to this email directly or view it on GitHub
#2120 (comment).

@thelinuxkid
Copy link
Contributor Author

Rebased. I am getting unrelated errors on TestFilePersistence (https://travis-ci.org/thelinuxkid/go-ipfs/jobs/102521435#L252) and TestMultipleDirs (https://travis-ci.org/thelinuxkid/go-ipfs/jobs/102521435#L255). I will try to take a look at them this weekend.

@rht
Copy link
Contributor

rht commented Jan 23, 2016

@thelinuxkid the travis-ci err has been logged in #2002. I think this just need a rerun.

if n.Identity == "" || n.Peerstore == nil {
return errors.New("loaded private key out of order.")
if n.PrivateKey != nil {
return nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im curious why this had to be added? is there a race condition somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See @whyrusleeping's response to my first comment in this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah makes sense thanks

@jbenet
Copy link
Member

jbenet commented Jan 24, 2016

  • I re-ran tests, the travis go one keeps failing... odd.
  • Ooof, the times bumped up to 14min in linux and 7min in osx. rough!
  • It is very useful to have fuse though.
  • It's silly for travis-ci to force install every time, i think we should contact them about using dockerfiles so that the FUSE image can be cached forever.

@whyrusleeping and @chriscool could you take a look at these run outputs and confirm all looks good? (i.e. testing fuse as we expect it to be tested, etc):

@chriscool
Copy link
Contributor

My concern with this is that docker is not available so t0300 is not run:

*** t0300-docker-image.sh ***

./t0300-docker-image.sh

lib/test-lib.sh: line 45: type: docker: not found

# TEST_VERBOSE=1

# TEST_NO_FUSE=

# TEST_EXPENSIVE=1

�[36m1..0 # SKIP skipping docker tests, docker not available�(B�[m

Maybe we could have the tests run in both configurations so that both fuse tests and tests with docker can be run?

@jbenet
Copy link
Member

jbenet commented Jan 24, 2016

Maybe we could have the tests run in both configurations so that both fuse tests and tests with docker can be run?

yeah that sounds good. do we know how to set this up?

i wish we could pay travis money to add to the number of workers we get. sigh. they keep not letting me pay them!!

@chriscool
Copy link
Contributor

@jbenet I am not sure it is possible to have both kind of tests.

I am testing things on this PR to see if it's possible: #2241

@jbenet
Copy link
Member

jbenet commented Jan 24, 2016

@chriscool we should be able to have two different builders with different env flags. the ipfsbot account should let you experiment with travis, it has perms there.

@chriscool
Copy link
Contributor

@jbenet according to the result of PR #2241, it looks like when "dist: trusty" is set in the .travis.yml file, then docker is not available. So it looks like we might just want to remove "dist: trusty" from .travis.yml, and then we don't need two different builders. One sharness build will test both fuse and docker.

@thelinuxkid was there a reason you added, or uncommented, "dist: trusty" into .travis.yml?

@thelinuxkid
Copy link
Contributor Author

@jbenet fuse was added on trusty build: #2023, travis-ci/travis-ci#1100 (comment)

@thelinuxkid
Copy link
Contributor Author

FWIT fuse module is already installed on trusty: thelinuxkid@764e756, https://travis-ci.org/thelinuxkid/go-ipfs/builds/104456298.

@chriscool
Copy link
Contributor

@thelinuxkid yeah but it looks like if we don't specify "dist: trusty", we can have both fuse and also docker.

@thelinuxkid
Copy link
Contributor Author

I am pro containers. Specially if it eliminates the need to depend on travis' build images. But, I know nothing of its support in travis. Having said that, I removed the explicit trusty distribution (https://travis-ci.org/thelinuxkid/go-ipfs/builds/104545197) and travis still used trusty (https://travis-ci.org/thelinuxkid/go-ipfs/jobs/104545198#L14). Is that some sort of confusion...bug...is trusty now the default?

@thelinuxkid
Copy link
Contributor Author

BTW except for #2214 (which is resolved but needs to be implemented) and stalls, I am not getting any more travis errors.

@chriscool
Copy link
Contributor

@thelinuxkid yeah, it looks like Docker works now on Travis. So now this PR LGTM! Thanks!

@chriscool chriscool added the RFM label Jan 27, 2016
@chriscool
Copy link
Contributor

@whyrusleeping or @jbenet could you take a look at merging this?
The test failures are not related and have been seen elsewhere.

@rht
Copy link
Contributor

rht commented Jan 27, 2016

This needs a rebase from @thelinuxkid

@jbenet
Copy link
Member

jbenet commented Jan 27, 2016

Or Anyone can rebase and PR again
On Wed, Jan 27, 2016 at 04:40 rht notifications@github.com wrote:

This needs a rebase from @thelinuxkid https://github.com/thelinuxkid


Reply to this email directly or view it on GitHub
#2120 (comment).

@thelinuxkid
Copy link
Contributor Author

Rebased. Travis tests failed in a weird way. Rerunning. Will report.

@chriscool
Copy link
Contributor

There is this go test failure:

--- FAIL: TestFilePersistence (0.22s)
    ipns_test.go:189: Closed, opening new fs
    ipns_test.go:195: open /tmp/fusetest063103241/local/atestfile: no such file or directory
--- FAIL: TestMultipleDirs (0.24s)
    ipns_test.go:206: make a top level dir
    ipns_test.go:212: write a file in it
    ipns_test.go:217: sub directory
    ipns_test.go:222: file in that subdirectory
    ipns_test.go:228: closing mount, then restarting
    ipns_test.go:88: stat /tmp/fusetest677160995/local/test1: no such file or directory
FAIL
FAIL    github.com/ipfs/go-ipfs/fuse/ipns   7.021s

but it's known and not related.

@thelinuxkid
Copy link
Contributor Author

@chriscool yep. It's addressed here: #2214. Everything else looks good.

@whyrusleeping
Copy link
Member

this looks good to me, mind rebasing on latest master?

@chriscool
Copy link
Contributor

@thelinuxkid do you want me to rebase on master?

@thelinuxkid
Copy link
Contributor Author

@chriscool just rebased. Thanks.

@thelinuxkid
Copy link
Contributor Author

Still getting the test failures from #2214. I will try to work on that tomorrow and/or Friday if no one else can pick it up.

@rht
Copy link
Contributor

rht commented Feb 6, 2016

Why did the #2214 happen twice specifically here?

@thelinuxkid
Copy link
Contributor Author

@rht Because this enables the fuse tests

@thelinuxkid
Copy link
Contributor Author

I rebased and the travis tests were failing. So I reset --hard upstream/master and pushed to see if I had broken anything. That closed this PR because Github detected no new commits. I pushed again with the changes this time but it won't reopen. Sorry. I should have tested locally instead of relaying on travis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/test Testing work RFM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants