Skip to content
This repository was archived by the owner on Sep 9, 2020. It is now read-only.

internal/fs: Remove symlink cycle from testdata #1412

Closed

Conversation

kamalmarhubi
Copy link

What does this do / why do we need it?

Some tools get unhappy with a symlink cycle, and the test is not
verifying anything related to the directory symlink being a cycle.

What should your reviewer look out for in this PR?

Make sure tests pass on Windows.

Some tools get unhappy with a symlink cycle, and the test is not
verifying anything related to the directory symlink being a cycle.
@ibrasho
Copy link
Collaborator

ibrasho commented Dec 15, 2017

Tests were failing on macOS. I restarted the build to make sure it's not a random hiccup.

I'm still not sure why this change is needed... Can you elaborate more on which tools are failing?

@kamalmarhubi
Copy link
Author

I think it was bazel: it refused to do anything at all. I'm not at a computer right now, so can't verify.

@ixdy
Copy link

ixdy commented Jan 25, 2018

I'm attempting to vendor dep and ran into this issue (it seems like it doesn't prune the testdata/ directory, unfortunately). It's causing bazel to explode, yes.

$ bazel test ...
ERROR: infinite symlink expansion detected
[start of symlink chain]
.../src/k8s.io/test-infra/vendor/github.com/golang/dep/internal/fs/testdata/symlinks/dir-symlink
.../src/k8s.io/test-infra/vendor/github.com/golang/dep/internal/fs/testdata
[end of symlink chain]
ERROR: Failed to get information about path, for vendor/github.com/golang/dep/internal/fs/testdata/symlinks/dir-symlink, skipping: Infinite symlink expansion
ERROR: Infinite symlink expansion

@sdboyer
Copy link
Member

sdboyer commented Jan 26, 2018

@carolynvs the stall appears to be happening in one of the internal/importers/base tests - do you have any idea why rearranging this symlink could be causing test hangs over there? a quick skim does not reveal any obvious links between these packages.

@carolynvs
Copy link
Collaborator

@sdboyer Nothing comes to mind, and I am unable to reproduce on my mac. Looks like these three tests are running at the same time, though I don't quite get why, since I don't have parallel tests enabled for the importers.

TestBaseImporter_ImportProjects
TestUnreachableSource
TestSignalHandling

@sdboyer
Copy link
Member

sdboyer commented Jul 23, 2018

i'm gonna just do this directly (a test also has to be updated).

@sdboyer sdboyer closed this Jul 23, 2018
@sdboyer
Copy link
Member

sdboyer commented Jul 24, 2018

done, in 7ca88e9 . idk what those test failures were, but they were unrelated.

@kamalmarhubi
Copy link
Author

Many thanks @sdboyer. I'd noticed this PR in my pending list last week and was going to revisit or close. Glad it's been sorted!

@sdboyer sdboyer added this to the v0.5.0 milestone Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants