Skip to content
This repository has been archived by the owner on Mar 31, 2022. It is now read-only.

create a test that fails to demonstrate issue #75 #82

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ryangardner
Copy link

Note: in its current form, this test fails... I haven't worked on a fix for it, but figured an IT that failed might make it easier to fix the issue reported in #75

@ryangardner
Copy link
Author

Hmm... nevermind. this test doesn't demonstrate this issue yet. I'll try to improve it so it does.

@ryangardner
Copy link
Author

This latest PR also adds a simple fix for the issue identified... the test metadata method tries to copy everything from the docker info directory over - if that directory is used for something else and has folders in it, the copy will fail.

It seems like only copying the set of expected files over would be a better behavior, because it's hard to detect if someone is putting files in that directory that shouldn't be there - but if there is a directory there in this PR it will be detected and a suggestion on how to configure the plugin will be logged at the warning level

Copy link
Member

@mattnworb mattnworb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR and for aiming to reproduce this in a test case.

I'd prefer if this issue was addressed by validating at the start of the plugin execution that contextDirectory and dockerInfoDirectory were not colliding or overlapping, as I think that would probably cause additional headaches or weird behavior beyond the writeTestMetadata exception.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants