Well it passed here so I guess that's good ;)

It would be the best to have at least a test case for LogConfig in CreateContainerCmdImplTest. Any chance?

That's the thing: no matter what one puts in for values the command to create a container succeeds. If I put in "fake-log-driver" for type docker will not complain and just default to log-driver none. If I put in configs that have nothing to do with the type docker will just ignore them.

So with that in mind I'm not sure what we could even test.

On a side-note I've made the requested changes and fleshed out the support a bit more for this option to be more inline with what's already provided.

I see failures with integration testing, both locally and through circle-ci, but they do not look related to my changes.

I think a basic test could create a container with a valid log config (like json-file) and inspect the container afterwards to check if the settings are accepted by the docker host.

Don't worry about failed intergration tests. CircleCI uses a quite old docker version where log config is not supported.

I've added the test 'CreateContainerCmdImplTest.createContainerWithLogConfig' which sets LogConfig to 'none' and inspects the reponse to ensure that is what is actually set. Works here against my local docker instance. I added the ' @test(groups = "ignoreInCircleCi")' annotation as I assume we don't want this run on CircleCI due to it using an older docker version.

@cdancy I've modified your code so the logging type is an enum now. Thanks for your contribution!

Good call. I was thinking of doing the same but wasn't sure if we should force the user to pass in the Enum on the LogConfig constructor/setters or do a check to ensure the type was supported.