-
-
Notifications
You must be signed in to change notification settings - Fork 9
implementation: Test for symlinked files #12
Conversation
implementation = Trackler::Implementation.new('animal', URL, problem, PATH) | ||
|
||
expected = "This should get included in fish.\n" | ||
assert_equal expected, archive_contents(implementation.zip, 'included-via-symlink.txt') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessing it via the archive is redundant, you should be able to do:
assert_equal expected, implementation.files['included-via-symlink.txt']
@@ -0,0 +1 @@ | |||
This should get included in fish. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name the directory common
or shared
or .meta
rather than symlinks
?
It would be good to be able to standardise across tracks, and it could be good to indicate that this is not an implementation directory. So my preference is probably for .meta
matching the ignored directory name within an implementation. See: #4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose shared
.
Never mind, I chose .meta
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could maybe call it .meta/shared
in the track repository, that's completely up to you.
Having it be just .meta
in the test seems reasonable.
@@ -91,4 +99,13 @@ def archive_filenames(zip) | |||
end | |||
files | |||
end | |||
|
|||
def archive_contents(zip, filename) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unneeded now.
We have a use case in exercism/haskell#442 where we would like to symlink files. We have one common file that will get delivered with each exercise, and we would only like to have to maintain one instance of it in the repo, while symlinking it into all exercise directories. This is a separate case than https://github.com/exercism/x-api/issues/29 because that talks about files that are not associated with any particular exercise - their destination is a directory outside of any exercise directory, whereas our destination is inside each exercise directory. For this case to work for us, we need trackler to correctly follow symlinks present in an implementation. It currently does. In adding this test, we request that this behaviour in the face of a symlink not change in future trackler versions, for we have started to depend on it as of exercism/haskell#443
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
I like the idea of adding a test to make it explicit that symlinks are supported.
This is great—I really like the addition of this test. |
This is failing on CI after being merged, but it seemed to be passing just fine in the PR: https://travis-ci.org/exercism/trackler/builds/188348834#L668 |
I'm going to revert the PR for now. |
fixtures: Move exercises into exercises directory; reapply #12
We have a use case in exercism/haskell#442
where we would like to symlink files.
We have one common file that will get delivered with each exercise, and
we would only like to have to maintain one instance of it in the repo,
while symlinking it into all exercise directories.
This is a separate case than https://github.com/exercism/x-api/issues/29
because that talks about files that are not associated with any
particular exercise - their destination is a directory outside of any
exercise directory, whereas our destination is inside each exercise
directory.
For this case to work for us, we need trackler to correctly follow
symlinks present in an implementation. It currently does. In adding this
test, we request that this behaviour in the face of a symlink not
change in future trackler versions, for we have started to depend on it
as of exercism/haskell#443