Modify tests for previous-image flag (#897)

Signed-off-by: Ujjwal Goyal <importujjwal@gmail.com>

Author: importhuman

acceptance/acceptance_test.go

@@ -1764,45 +1764,6 @@ func testAcceptance(
 						})
 					})
 
-					// when("--previous-image", func() {
-					// 	var prevImage string
-					// 	it.Before(func() {
-					// 		prevImage = fmt.Sprintf("%s", repoName)
-					// 	})
-
-					// 	it("creates image and previous image on the registry", func() {
-					// 		buildArgs := []string{
-					// 			repoName,
-					// 			"-p", filepath.Join("testdata", "mock_app"),
-					// 			"--publish",
-					// 			"--previous-image",
-					// 			prevImage,
-					// 		}
-					// 		if imageManager.HostOS() != "windows" {
-					// 			buildArgs = append(buildArgs, "--network", "host")
-					// 		}
-
-					// 		output := pack.RunSuccessfully("build", buildArgs...)
-					// 		assertions.NewOutputAssertionManager(t, output).ReportsSuccessfulImageBuild(repoName)
-
-					// 		prevImageRef, err := name.ParseReference(prevImage, name.WeakValidation)
-					// 		assert.Nil(err)
-
-					// 		t.Log("checking that registry has contents")
-					// 		assertImage.CanBePulledFromRegistry(repoName)
-					// 		if imageManager.HostOS() == "windows" {
-					// 			// Cache images are automatically Linux container images, and therefore can't be pulled
-					// 			// and inspected correctly on WCOW systems
-					// 			//https://github.com/buildpacks/lifecycle/issues/529
-					// 			imageManager.PullImage(prevImageRef.Name(), registryConfig.RegistryAuth())
-					// 		} else {
-					// 			assertImage.CanBePulledFromRegistry(prevImageRef.Name())
-					// 		}
-
-					// 		defer imageManager.CleanupImages(prevImageRef.Name())
-					// 	})
-					// })
-
 					when("ctrl+c", func() {
 						it("stops the execution", func() {
 							var buf = new(bytes.Buffer)

internal/build/lifecycle_execution.go

@@ -188,11 +188,17 @@ func (l *LifecycleExecution) Create(ctx context.Context, publish bool, dockerHos
 		flags = append(flags, "-gid", strconv.Itoa(l.opts.GID))
 	}
 
+	if l.opts.Image == nil {
+		return errors.New("image can't be nil")
+	}
+
+	// validate image name
+	image, err := name.ParseReference(l.opts.Image.Name(), name.WeakValidation)
+	if err != nil {
+		return fmt.Errorf("invalid image name: %s", err)
+	}
+
 	if l.opts.PreviousImage != "" {
-		image, err := name.ParseReference(l.opts.Image.Name(), name.WeakValidation)
-		if err != nil {
-			return fmt.Errorf("invalid image name: %s", err)
-		}
 		prevImage, err := name.ParseReference(l.opts.PreviousImage, name.WeakValidation)
 		if err != nil {
 			return fmt.Errorf("invalid previous image name: %s", err)

internal/build/lifecycle_execution_test.go

@@ -840,6 +840,20 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
 			})
 		})
 
+		when("image is invalid", func() {
+			it("errors", func() {
+				var imageName name.Tag
+				imageName, err := name.NewTag("/x/y/?!z", name.WeakValidation)
+				h.AssertError(t, err, "repository can only contain the runes `abcdefghijklmnopqrstuvwxyz0123456789_-./`")
+				lifecycle := newTestLifecycleExec(t, true, func(options *build.LifecycleOptions) {
+					options.Image = imageName
+				})
+				fakePhaseFactory := fakes.NewFakePhaseFactory()
+				err = lifecycle.Create(context.Background(), false, "", false, "test", "test", "test", fakeBuildCache, fakeLaunchCache, []string{}, []string{}, fakePhaseFactory)
+				h.AssertError(t, err, "invalid image name")
+			})
+		})
+
 		when("-previous-image is used", func() {
 			when("previous-image is invalid", func() {
 				it("errors", func() {
@@ -852,25 +866,10 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
 					})
 					fakePhaseFactory := fakes.NewFakePhaseFactory()
 					err = lifecycle.Create(context.Background(), false, "", false, "test", "test", "test", fakeBuildCache, fakeLaunchCache, []string{}, []string{}, fakePhaseFactory)
-					h.AssertError(t, err, fmt.Sprintf("%s", err))
+					h.AssertError(t, err, "invalid previous image name")
 				})
 			})
 
-			// when("image is invalid", func() {
-			// 	it("errors", func() {
-			// 		var imageName name.Tag
-			// 		imageName, err := name.NewTag("/x/y/z", name.WeakValidation)
-			// 		h.AssertNil(t, err)
-			// 		lifecycle := newTestLifecycleExec(t, true, func(options *build.LifecycleOptions) {
-			// 			options.PreviousImage = "previous-image"
-			// 			options.Image = imageName
-			// 		})
-			// 		fakePhaseFactory := fakes.NewFakePhaseFactory()
-			// 		err = lifecycle.Create(context.Background(), false, "", false, "test", "test", "test", fakeBuildCache, fakeLaunchCache, []string{}, []string{}, fakePhaseFactory)
-			// 		h.AssertError(t, err, fmt.Sprintf("%s", err))
-			// 	})
-			// })
-
 			when("--publish is false", func() {
 				it("passes previous-image to creator", func() {
 					var imageName name.Tag

internal/commands/build_test.go

@@ -734,14 +734,6 @@ builder = "my-builder"
 						h.AssertNil(t, command.Execute())
 					})
 				})
-
-				// when("image and previous-image are not in same registry", func() {
-				// 	it("fails", func() {
-				// 		command.SetArgs([]string{"--builder", "my-builder", "index.docker.io/some/image:latest", "--previous-image", "example.io/some/previous-image", "--publish"})
-				// 		err := command.Execute()
-				// 		h.AssertError(t, err, "image and previous-image should be in the same registry when --publish is true")
-				// 	})
-				// })
 			})
 		})
 	})