Restore code reverted by bad rebase

[cpuguy83]

b70e037 was originally based on a very old branch.
A rebase was required and was not properly reviewed. Many important changes were accidentially reverted as part of the commit.

This change brings those changes back.

[Copilot]

Pull request overview

This PR restores code that was accidentally reverted during a bad rebase of commit b70e0372. The changes primarily refactor a monolithic test into focused sub-tests, add new test coverage for gomod, cache management, and deb-specific features, and fix a missing DnfInstallWithConstraints option in RPM test dependency installation.

Changes:

• Refactors the large monolithic container test in test/linux_target_test.go into focused sub-tests (build_steps, sources, artifacts, tests, container), adds a testLinuxSpec helper to reduce boilerplate, adds testDepsOnly for deps-only target testing, adds extensive new test coverage for gomod replace directives, cache key management, deb-specific features (dpkg debug, upgrades, excludes config), and corrects multiple test targets from Container to Package where container-specific features aren't needed.
• Fixes targets/linux/rpm/distro/pkg.go by adding DnfInstallWithConstraints(opts) to test dependency installation and hoisting the progress group append before the closure.
• Removes the obsolete mariner2 entry from the artifact capabilities test targets map and adds a missing scanner.Err() check.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File	Description
test/linux_target_test.go	Major test refactoring: splits monolithic test into focused sub-tests, adds testLinuxSpec and testDepsOnly helpers, adds new gomod/cache/deb tests, corrects Container→Package targets, removes mariner2, adds scanner error check
targets/linux/rpm/distro/pkg.go	Adds DnfInstallWithConstraints(opts) to test dep install options, moves ProgressGroup append before the closure

[invidian]

@cpuguy83 so the net changes from #898 are now the following it seems (with my moving suggestion applied). Does this look correct to you?

diff --git test/linux_target_test.go test/linux_target_test.go
index 7c67b317..b4bb2016 100644
--- test/linux_target_test.go
+++ test/linux_target_test.go
@@ -11,6 +11,7 @@ import (
 	"io"
 	"io/fs"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -26,7 +27,6 @@ import (
 	moby_buildkit_v1_frontend "github.com/moby/buildkit/frontend/gateway/pb"
 	"github.com/moby/go-archive/compression"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
-	pkgerrors "github.com/pkg/errors"
 	"github.com/project-dalec/dalec"
 	"github.com/project-dalec/dalec/frontend"
 	"github.com/project-dalec/dalec/frontend/pkg/bkfs"
@@ -4410,173 +4410,259 @@ func testLinuxPackageTestsFail(ctx context.Context, t *testing.T, cfg testLinuxC
 		t.Parallel()
 		ctx := startTestSpan(ctx, t)

-		spec := &dalec.Spec{
-			Name:        "test-package-tests",
-			Version:     "0.0.1",
-			Revision:    "42",
-			Description: "Testing package tests",
-			License:     "MIT",
-			Dependencies: &dalec.PackageDependencies{
-				Test: map[string]dalec.PackageConstraints{"bash": {}},
-			},
-			Image: &dalec.ImageConfig{
-				Post: &dalec.PostInstall{
-					Symlinks: map[string]dalec.SymlinkTarget{
-						"/usr/bin/a-thing-for-symlinking": {Paths: []string{"/some_symlink1"}},
-					},
+		newSpec := func() *dalec.Spec {
+			return &dalec.Spec{
+				Name:        "test-package-tests",
+				Version:     "0.0.1",
+				Revision:    "42",
+				Description: "Testing package tests",
+				License:     "MIT",
+				Dependencies: &dalec.PackageDependencies{
+					Test: map[string]dalec.PackageConstraints{"bash": {}},
 				},
-			},
-			Build: dalec.ArtifactBuild{
-				Steps: []dalec.BuildStep{
-					{
-						Command: "touch a-thing-for-symlinking",
+				Image: &dalec.ImageConfig{
+					Post: &dalec.PostInstall{
+						Symlinks: map[string]dalec.SymlinkTarget{
+							"/usr/bin/a-thing-for-symlinking": {Paths: []string{"/some_symlink1"}},
+						},
 					},
 				},
-			},
-			Artifacts: dalec.Artifacts{
-				Binaries: map[string]dalec.ArtifactConfig{
-					"a-thing-for-symlinking": {},
-				},
-				Links: []dalec.ArtifactSymlinkConfig{
-					{
-						Source: "/usr/bin/a-thing-for-symlinking",
-						Dest:   "/some_symlink2",
+				Build: dalec.ArtifactBuild{
+					Steps: []dalec.BuildStep{
+						{
+							Command: "touch a-thing-for-symlinking",
+						},
 					},
-					{
-						Source: "/not-a-real-path3",
-						Dest:   "/some_symlink3",
+				},
+				Artifacts: dalec.Artifacts{
+					Binaries: map[string]dalec.ArtifactConfig{
+						"a-thing-for-symlinking": {},
 					},
-					{
-						Source: "/not-a-real-path4",
-						Dest:   "/some_symlink4",
+					Links: []dalec.ArtifactSymlinkConfig{
+						{
+							Source: "/usr/bin/a-thing-for-symlinking",
+							Dest:   "/some_symlink2",
+						},
+						{
+							Source: "/not-a-real-path3",
+							Dest:   "/some_symlink3",
+						},
+						{
+							Source: "/not-a-real-path4",
+							Dest:   "/some_symlink4",
+						},
 					},
 				},
-			},
-			Tests: []*dalec.TestSpec{
-				{
-					Name: "Test that tests fail the build",
+			}
+		}
+
+		type testCase struct {
+			err          error
+			test         *dalec.TestSpec
+			isBuildError bool
+		}
+
+		// Because buildkit solves are fail-fast (the build is cancelled on the first failure), the only way to deterministically test
+		// multiple failure cases is to have separate builds for each expected failure.
+		expectedErrs := []testCase{
+			{
+				err: (&dalec.CheckOutputError{Path: "/non-existing-file", Kind: dalec.CheckFileNotExistsKind, Expected: "exists=true", Actual: "exists=false"}),
+				test: &dalec.TestSpec{
+					Name: "Test that non-existing file with no options fails the build",
 					Files: map[string]dalec.FileCheckOutput{
 						"/non-existing-file": {},
 					},
 				},
-				{
+			},
+			{
+				err: (&dalec.CheckOutputError{Path: "/", Kind: dalec.CheckFilePermissionsKind, Expected: "-rw-r--r--", Actual: "-rwxr-xr-x"}),
+				test: &dalec.TestSpec{
 					Name: "Test that permissions check fails the build",
 					Files: map[string]dalec.FileCheckOutput{
 						"/": {Permissions: 0o644, IsDir: true},
 					},
 				},
-				{
+			},
+			{
+				err: (&dalec.CheckOutputError{Path: "/", Kind: dalec.CheckFileIsDirKind, Expected: "is_dir=false", Actual: "is_dir=true"}),
+				test: &dalec.TestSpec{
 					Name: "Test that dir check fails the build",
 					Files: map[string]dalec.FileCheckOutput{
 						"/": {IsDir: false},
 					},
 				},
-				{
-					Name: "Test that command exiting non-zero fails the build",
+			},
+			{
+				err: (&dalec.CheckOutputError{Path: "/some_symlink1", Kind: dalec.CheckFileLinkTargetPathKind, Expected: "/not-a-real-path1", Actual: "/usr/bin/a-thing-for-symlinking"}),
+				test: &dalec.TestSpec{
+					Name: "Test that image post symlink target check fails the build",
+					Files: map[string]dalec.FileCheckOutput{
+						"/some_symlink1": {
+							LinkTarget: "/not-a-real-path1",
+						},
+					},
+				},
+			},
+			{
+				err: (&dalec.CheckOutputError{Path: "/some_symlink2", Kind: dalec.CheckFileLinkTargetPathKind, Expected: "/not-a-real-path2", Actual: "/usr/bin/a-thing-for-symlinking"}),
+				test: &dalec.TestSpec{
+					Name: "Test that artifact symlink target check fails the build",
+					Files: map[string]dalec.FileCheckOutput{
+						"/some_symlink2": {
+							LinkTarget: "/not-a-real-path2",
+						},
+					},
+				},
+			},
+			{
+				err: (&dalec.CheckOutputError{Path: "/some_symlink3", Kind: dalec.CheckFileNotExistsKind, Expected: "exists=true", Actual: "exists=false"}),
+				test: &dalec.TestSpec{
+					Name: "Test that artifact symlink to non-existing file check fails the build",
+					Files: map[string]dalec.FileCheckOutput{
+						"/some_symlink3": {},
+					},
+				},
+			},
+			{
+				err: (&dalec.CheckOutputError{Path: "/some_symlink4", Kind: dalec.CheckFileLinkTargetPathKind, Expected: "/incorrect-target", Actual: "/not-a-real-path4"}),
+				test: &dalec.TestSpec{
+					Name: "Test that artifact symlink nofollow link target check fails the build",
+					Files: map[string]dalec.FileCheckOutput{
+						"/some_symlink4": {
+							NoFollow:   true,
+							LinkTarget: "/incorrect-target",
+						},
+					},
+				},
+			},
+			{
+				err:          &moby_buildkit_v1_frontend.ExitError{ExitCode: 42, Err: fmt.Errorf("step did not complete successfully")},
+				isBuildError: true,
+				test: &dalec.TestSpec{
+					Name: "Test that command exit code check fails the build",
 					Steps: []dalec.TestStep{
 						{Command: "/bin/sh -ec 'exit 42'"},
 					},
 				},
-				{
-					Name: "Test that command giving the wrong output fails the build",
+			},
+			{
+				err: (&dalec.CheckOutputError{Path: "stdout", Kind: dalec.CheckOutputEqualsKind, Expected: "stdout not hello", Actual: "hello\n"}),
+				test: &dalec.TestSpec{
+					Name: "Test that stdout check fails the build",
 					Steps: []dalec.TestStep{
 						{
 							Command: "/bin/sh -ec 'echo hello'",
 							Stdout: dalec.CheckOutput{
 								Equals: "stdout not hello",
 							},
-							Stderr: dalec.CheckOutput{
-								Equals: "stderr not hello",
-							},
 						},
 					},
 				},
-				{
-					Name: "Test that incorrect symlink path targets fail the build",
-					Files: map[string]dalec.FileCheckOutput{
-						"/some_symlink1": {
-							LinkTarget: "/not-a-real-path1",
-						},
-						"/some_symlink2": {
-							LinkTarget: "/not-a-real-path2",
+			},
+			{
+				err: (&dalec.CheckOutputError{Path: "stderr", Kind: dalec.CheckOutputEqualsKind, Expected: "stderr not hello", Actual: "hello\n"}),
+				test: &dalec.TestSpec{
+					Name: "Test that stderr check fails the build",
+					Steps: []dalec.TestStep{
+						{
+							Command: "/bin/sh -ec 'echo hello >&2'",
+							Stderr: dalec.CheckOutput{
+								Equals: "stderr not hello",
+							},
 						},
-						// check that a symlink pointing to a non-existent path with NoFollow=false should error (with a CheckFileNotExistsKind)
-						"/some_symlink3": {},
-						// And then that we can check symlink target with NoFollow=true when the target doesn't exist
-						"/some_symlink4": {NoFollow: true, LinkTarget: "/incorrect-target"},
 					},
 				},
 			},
 		}

-		expectedErrors := []string{
-			(&dalec.CheckOutputError{Path: "/non-existing-file", Kind: dalec.CheckFileNotExistsKind, Expected: "exists=true", Actual: "exists=false"}).Error(),
-			(&dalec.CheckOutputError{Path: "/", Kind: dalec.CheckFilePermissionsKind, Expected: "-rw-r--r--", Actual: "-rwxr-xr-x"}).Error(),
-			(&dalec.CheckOutputError{Path: "/", Kind: dalec.CheckFileIsDirKind, Expected: "ModeFile", Actual: "ModeDir"}).Error(),
-			(&dalec.CheckOutputError{Path: "/some_symlink1", Kind: dalec.CheckFileLinkTargetPathKind, Expected: "/not-a-real-path1", Actual: "/usr/bin/a-thing-for-symlinking"}).Error(),
-			(&dalec.CheckOutputError{Path: "/some_symlink2", Kind: dalec.CheckFileLinkTargetPathKind, Expected: "/not-a-real-path2", Actual: "/usr/bin/a-thing-for-symlinking"}).Error(),
-			(&dalec.CheckOutputError{Path: "/some_symlink3", Kind: dalec.CheckFileNotExistsKind, Expected: "exists=true", Actual: "exists=false"}).Error(),
-			(&dalec.CheckOutputError{Path: "/some_symlink4", Kind: dalec.CheckFileLinkTargetPathKind, Expected: "/incorrect-target", Actual: "/not-a-real-path4"}).Error(),
-			"step did not complete successfully: exit code: 42",
-			"stdout not hello",
-			"stderr not hello",
+		newLogFile := func(t *testing.T) *os.File {
+			t.Helper()
+			dir := t.TempDir()
+			f, err := os.OpenFile(filepath.Join(dir, "solve-status-log.txt"), os.O_CREATE|os.O_RDWR, 0o644)
+			assert.NilError(t, err)
+			t.Cleanup(func() { f.Close() })
+
+			return f
 		}

-		testForTarget := func(target string) func(t *testing.T) {
+		runTest := func(target string) func(t *testing.T) {
 			return func(t *testing.T) {
 				t.Parallel()
+				ctx := startTestSpan(ctx, t)

-				dir := t.TempDir()
-				f, err := os.OpenFile(filepath.Join(dir, "out.txt"), os.O_CREATE|os.O_RDWR, 0o600)
-				assert.NilError(t, err)
-				defer f.Close()
+				for _, tc := range expectedErrs {
+					t.Run(tc.test.Name, func(t *testing.T) {
+						t.Parallel()
+						ctx := startTestSpan(ctx, t)

-				consumeLogs := func(status *client.SolveStatus) {
-					if status == nil {
-						return
-					}
+						f := newLogFile(t)

-					for _, v := range status.Logs {
-						_, err := f.Write(v.Data)
-						assert.NilError(t, err)
-					}
-				}
+						var size int
+						solveStatusFn := testenv.WithSolveStatusFn(func(status *client.SolveStatus) {
+							if status == nil {
+								return
+							}

-				ctx := startTestSpan(ctx, t)
-				testEnv.RunTest(ctx, t, func(ctx context.Context, client gwclient.Client) {
-					sr := newSolveRequest(withSpec(ctx, t, spec), withBuildTarget(target))
-					_, err := client.Solve(ctx, sr)
-					assert.Assert(t, err != nil)
+							for _, v := range status.Logs {
+								if _, err := f.Write(v.Data); err != nil {
+									t.Error(err)
+								}
+								size += len(v.Data)
+							}
+						})

-					// Make sure the error is an exit error
-					var xErr *moby_buildkit_v1_frontend.ExitError
-					assert.Check(t, cmp.ErrorType(pkgerrors.Cause(err), xErr))
-				}, testenv.WithSolveStatusFn(consumeLogs))
+						spec := newSpec()
+						spec.Tests = []*dalec.TestSpec{tc.test}

-				_, err = f.Seek(0, 0)
-				assert.NilError(t, err)
+						testEnv.RunTest(ctx, t, func(ctx context.Context, client gwclient.Client) {
+							sr := newSolveRequest(withSpec(ctx, t, spec), withBuildTarget(target), withIgnoreCache(frontend.IgnoreCacheTestsKey))
+							_, err := client.Solve(ctx, sr)
+							assert.Assert(t, err != nil)

-				dt, err := io.ReadAll(f)
-				assert.NilError(t, err)
+							t.Logf("Build Error: %v", err)

-				for _, expectErr := range expectedErrors {
-					assert.Check(t, cmp.Contains(string(dt), expectErr))
-				}
+							var exErr *moby_buildkit_v1_frontend.ExitError
+							assert.Assert(t, errors.As(err, &exErr), "expected exit error, got: %v", err)
+
+							if !tc.isBuildError {
+								// the error we are looking for is in the build logs, not the exit error
+								return
+							}
+
+							assert.Equal(t, exErr.ExitCode, tc.err.(*moby_buildkit_v1_frontend.ExitError).ExitCode)
+						}, solveStatusFn)
+
+						if tc.isBuildError {
+							return
+						}
+
+						_, err := f.Seek(0, io.SeekStart)
+						assert.NilError(t, err)
+
+						dt, err := io.ReadAll(f)
+						assert.NilError(t, err)

-				if t.Failed() {
-					t.Log("---- Full build log ----\n" + string(dt))
+						if !bytes.Contains(dt, []byte(tc.err.Error())) {
+							t.Errorf("expected error not found in logs")
+							t.Logf("Expected error:\n\t%v", tc.err)
+						}
+					})
 				}
 			}
 		}

-		t.Run("package", testForTarget(cfg.Target.Package))
-		t.Run("container", testForTarget(cfg.Target.Package))
+		t.Run(path.Base(cfg.Target.Package), runTest(cfg.Target.Package))
+		t.Run(path.Base(cfg.Target.Container), runTest(cfg.Target.Container))
 	})

 	t.Run("positive test", func(t *testing.T) {
 		t.Parallel()
 		ctx := startTestSpan(baseCtx, t)

+		equalCheck := func(v string) dalec.CheckOutput {
+			return dalec.CheckOutput{Equals: v}
+		}
+
 		spec := &dalec.Spec{
 			Name:        "test-package-tests",
 			Version:     "0.0.1",
@@ -4629,6 +4715,24 @@ func testLinuxPackageTestsFail(ctx context.Context, t *testing.T, cfg testLinuxC
 						"/some_symlink3": {LinkTarget: "/not-a-real-file", NoFollow: true},
 					},
 				},
+				{
+					Name: "Test multiple commands with no fs changes",
+					Steps: []dalec.TestStep{
+						{Command: "/bin/sh -ec 'echo command one'"},
+						{Command: "/bin/sh -ec 'echo command two'"},
+						{Command: "/bin/sh -ec 'echo command three'"},
+						{Command: "/bin/sh -ec 'echo command four'"},
+					},
+				},
+				{
+					Name: "Test multiple commands with stdio checks",
+					Steps: []dalec.TestStep{
+						{Command: "/bin/sh -ec 'echo command one'", Stdout: equalCheck("command one\n")},
+						{Command: "/bin/sh -ec 'echo command two'"},
+						{Command: "/bin/sh -ec 'echo command three'", Stdout: equalCheck("command three\n")},
+						{Command: "/bin/sh -ec 'echo command four'"},
+					},
+				},
 				{
 					Name: "Test that test mounts work",
 					Files: map[string]dalec.FileCheckOutput{
@@ -4700,16 +4804,26 @@ func testLinuxPackageTestsFail(ctx context.Context, t *testing.T, cfg testLinuxC
 			},
 		}

-		testEnv.RunTest(ctx, t, func(ctx context.Context, client gwclient.Client) {
-			sr := newSolveRequest(withSpec(ctx, t, spec), withBuildTarget(cfg.Target.Package))
-			res := solveT(ctx, t, client, sr)
-			_, err := res.SingleRef()
-			assert.NilError(t, err)
+		t.Run(path.Base(cfg.Target.Package), func(t *testing.T) {
+			t.Parallel()
+			ctx = startTestSpan(baseCtx, t)
+			testEnv.RunTest(ctx, t, func(ctx context.Context, client gwclient.Client) {
+				sr := newSolveRequest(withSpec(ctx, t, spec), withBuildTarget(cfg.Target.Package), withIgnoreCache(frontend.IgnoreCacheTestsKey))
+				res := solveT(ctx, t, client, sr)
+				_, err := res.SingleRef()
+				assert.NilError(t, err)
+			})
+		})

-			sr = newSolveRequest(withSpec(ctx, t, spec), withBuildTarget(cfg.Target.Container))
-			res = solveT(ctx, t, client, sr)
-			_, err = res.SingleRef()
-			assert.NilError(t, err)
+		t.Run(path.Base(cfg.Target.Container), func(t *testing.T) {
+			t.Parallel()
+			ctx := startTestSpan(baseCtx, t)
+			testEnv.RunTest(ctx, t, func(ctx context.Context, client gwclient.Client) {
+				sr := newSolveRequest(withSpec(ctx, t, spec), withBuildTarget(cfg.Target.Container), withIgnoreCache(frontend.IgnoreCacheTestsKey))
+				res := solveT(ctx, t, client, sr)
+				_, err := res.SingleRef()
+				assert.NilError(t, err)
+			})
 		})
 	})
 }
@@ -5171,7 +5285,9 @@ func testDisableAutoRequire(ctx context.Context, t *testing.T, cfg targetConfig)
 		spec := newSimpleSpec()
 		spec.Artifacts = dalec.Artifacts{
 			Binaries: map[string]dalec.ArtifactConfig{
-				"test": {},
+				"test": {
+					SubPath: "dalec",
+				},
 			},
 		}

@@ -5320,6 +5436,8 @@ int main() {
 					}
 				}

+				assert.NilError(t, scanner.Err())
+
 				if spec.Artifacts.DisableAutoRequires {
 					assert.Check(t, !found, "auto-requires found: %s\n%s", path, buf)
 				} else {

[invidian]

nit: this and testLinuxSpec could be moved at the bottom of the file to smaller diff between revision prior to #898.

[cpuguy83]

Removed the azure mirror changes because they were ineffectual.
The actual fix commit is unchanged.