Skip to content
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

integ-cli: fix test requiring scratch #11196

Merged
merged 1 commit into from Mar 6, 2015
Merged

integ-cli: fix test requiring scratch #11196

merged 1 commit into from Mar 6, 2015

Conversation

@ahmetb
Copy link
Contributor

@ahmetb ahmetb commented Mar 6, 2015

TestRunCidFileCleanupIfEmpty fails on windows/mac because the test runs
the command docker run scratch and it gives the following error:

Unable to find image 'scratch:latest' locally
Pulling repository scratch
511136ea3c5a: Download complete
FATA[0004] 'scratch' is a reserved name

I am not entirely sure if this is a test issue or not but I had a quick
workaround by creating another image using FROM scratch and using that.

Signed-off-by: Ahmet Alp Balkan ahmetalpbalkan@gmail.com
cc: @LK4D4 @duglin @cpuguy83

@duglin
Copy link
Contributor

@duglin duglin commented Mar 6, 2015

just wondering, does using 'busybox' instead work?

@cpuguy83
Copy link
Contributor

@cpuguy83 cpuguy83 commented Mar 6, 2015

Yeah, scratch is no longer a real image anymore and can't be used to create a container. It is however a reserved name for Dockerfiles. Busybox should also "just work" here.

@duglin
Copy link
Contributor

@duglin duglin commented Mar 6, 2015

@ahmetalpbalkan can you please fix this test? But not for the reason you state :-)
First, can you fix it so that it checks to make sure the 'run' fails for the right reason. I suspect its failing now because of "scratch" and not for the missing cmd. We need to check the error msg.

Then once the test fails, for the right reason, and on all platforms not just windows, we can then s/scratch/busybox/.

@ahmetb
Copy link
Contributor Author

@ahmetb ahmetb commented Mar 6, 2015

@duglin @cpuguy83 in this case busybox is not the solution, because you can docker run busybox and it does not give the Error response from daemon: No command specified error.

I will add the string.contains("No command specified") check but I think since scratch still seems like a valid name in FROM directive and successfully gives an image that we can't run like docker run img, I will keep that part. wdyt?

@tiborvass
Copy link
Collaborator

@tiborvass tiborvass commented Mar 6, 2015

@ahmetalpbalkan there is an image called emptyfs that replaces scratch right before the daemon is started, let's use that instead.

@cpuguy83
Copy link
Contributor

@cpuguy83 cpuguy83 commented Mar 6, 2015

Oh, maybe I didn't understand the test, I see the comment just above the func declaration in the full view now :)
Yes, that makes sense.

TestRunCidFileCleanupIfEmpty fails on windows/mac because the test runs
the command `docker run scratch` and it gives the following error:

	Unable to find image 'scratch:latest' locally
	Pulling repository scratch
	511136ea3c5a: Download complete
	FATA[0004] 'scratch' is a reserved name

I am not entirely sure if this is a test issue or not but I had a quick
workaround by creating another image using `FROM scratch` and using that.

Signed-off-by: Ahmet Alp Balkan <ahmetalpbalkan@gmail.com>
@ahmetb
Copy link
Contributor Author

@ahmetb ahmetb commented Mar 6, 2015

fixed with emptyfs and err message check @tiborvass @cpuguy83

@tiborvass
Copy link
Collaborator

@tiborvass tiborvass commented Mar 6, 2015

LGTM

@duglin
Copy link
Contributor

@duglin duglin commented Mar 6, 2015

much better. LGTM

@tianon
Copy link
Member

@tianon tianon commented Mar 6, 2015

LGTM

tianon added a commit that referenced this pull request Mar 6, 2015
…leanupIfEmpty-fix

integ-cli: fix test requiring scratch
@tianon tianon merged commit f5538dc into moby:master Mar 6, 2015
1 check passed
1 check passed
janky Jenkins build Docker-PRs 2631 has succeeded
Details
@ahmetb ahmetb deleted the ahmetb:win-cli/TestRunCidFileCleanupIfEmpty-fix branch Mar 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.