-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Support Dockerfile.dockerignore #801
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# This dockerfile makes sure Dockerfile.dockerignore is working | ||
# If so then ignore_relative/foo should copy to /foo | ||
# If not, then this image won't build because it will attempt to copy three files to /foo, which is a file not a directory | ||
FROM scratch | ||
COPY ignore_relative/* /foo |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
# A .dockerignore file to make sure dockerignore support works | ||
ignore_relative/** | ||
!ignore_relative/foo |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -694,3 +694,49 @@ func Test_childDirInWhitelist(t *testing.T) { | |
}) | ||
} | ||
} | ||
|
||
func Test_correctDockerignoreFileIsUsed(t *testing.T) { | ||
type args struct { | ||
dockerfilepath string | ||
buildcontext string | ||
excluded string | ||
notExcluded string | ||
} | ||
tests := []struct { | ||
name string | ||
args args | ||
}{ | ||
{ | ||
name: "relative dockerfile used", | ||
args: args{ | ||
dockerfilepath: "../../integration/dockerfiles/Dockerfile_test_dockerignore_relative", | ||
buildcontext: "../../integration/", | ||
excluded: "ignore_relative/bar", | ||
notExcluded: "ignore_relative/foo", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think we should test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. very good point, yes! |
||
}, | ||
}, | ||
{ | ||
name: "context dockerfile is used", | ||
args: args{ | ||
dockerfilepath: "../../integration/dockerfiles/Dockerfile_test_dockerignore", | ||
buildcontext: "../../integration/", | ||
excluded: "ignore/bar", | ||
notExcluded: "ignore/foo", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should test |
||
}, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
excluded = nil | ||
if err := GetExcludedFiles(tt.args.dockerfilepath, tt.args.buildcontext); err != nil { | ||
t.Fatal(err) | ||
} | ||
if excl := excludeFile(tt.args.excluded, tt.args.buildcontext); !excl { | ||
t.Errorf("'%v' not excluded as expected", tt.args.excluded) | ||
} | ||
if excl := excludeFile(tt.args.notExcluded, tt.args.buildcontext); excl { | ||
t.Errorf("'%v' excluded against expectation", tt.args.notExcluded) | ||
} | ||
}) | ||
} | ||
} |
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.
Can we remove this from integration test and rename this to "Dockerfile_dockeringore_relative"
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.
@tejal29 I'm not 100% sure how things work: if the file doesn't have the word
test
in it, then it won't be included in integration tests or is there more to do?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.
ah you answered below, thx :)