fix: missing path segments after untarring#514
Conversation
There was a problem hiding this comment.
Thanks for the fix, I think the main change looks good. I left one comment about the test changes to simplify things.
In regards to eliminating Docker, I'm not sure how we could do that, since Docker is a first-class citizen in stereoscope and we need to test the functionality. We could maybe separate some of the tests to a docker integration suite, but we use docker quite a lot in our tools' testing to build test fixtures and such, if there were simple alternatives, I'd love to know!
It looks like your commits need to be Signed-Off
pkg/file/tarutil_test.go
Outdated
| if tt.wantErr == nil { | ||
| tt.wantErr = require.NoError | ||
| } | ||
| root := t.TempDir() |
There was a problem hiding this comment.
Instead of requiring passing the root dir everywhere, could you just use afero.NewBasePathFs(osFs, root)?
There was a problem hiding this comment.
Ah, didn't know about this, thanks!
Changed 👌
25c71e0 to
b39f26a
Compare
chore: use the real filesystem for tests, as the MemMapFs is implemented differently Signed-off-by: Chris Werner Rau <cwrau@cwrau.info>
b39f26a to
0d4686f
Compare
|
Thanks for the contribution @cwrau |
Fixes #492
Also adjusted the tarutil tests to use the real filesystem instead of MemMapFs,
as that just creates missing paths as opposed to erroring out, which is what the real filesystem does.
Also, you should think about either making the tests not dependent on
docker, as not everyone has it installed, such as me.