diff --git a/go/cmd/dolt/commands/cherry-pick.go b/go/cmd/dolt/commands/cherry-pick.go index 5cc82c175c4..735883d0d64 100644 --- a/go/cmd/dolt/commands/cherry-pick.go +++ b/go/cmd/dolt/commands/cherry-pick.go @@ -26,6 +26,7 @@ import ( "github.com/dolthub/dolt/go/cmd/dolt/errhand" "github.com/dolthub/dolt/go/libraries/doltcore/branch_control" "github.com/dolthub/dolt/go/libraries/doltcore/env" + "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" "github.com/dolthub/dolt/go/libraries/utils/argparser" "github.com/dolthub/dolt/go/store/util/outputpager" eventsapi "github.com/dolthub/eventsapi_schema/dolt/services/eventsapi/v1alpha1" @@ -51,6 +52,12 @@ var ErrCherryPickConflictsOrViolations = errors.NewKind("error: Unable to apply "To undo all changes from this cherry-pick operation, use `dolt cherry-pick --abort`.\n" + "For more information on handling conflicts, see: https://docs.dolthub.com/concepts/dolt/git/conflicts") +var ErrCherryPickVerificationFailed = errors.NewKind("error: Commit verification failed. Your changes are staged " + + "in the working set. Fix the failing tests, use `dolt add` to stage your changes, then " + + "`dolt cherry-pick --continue` to complete the cherry-pick.\n" + + "To undo all changes from this cherry-pick operation, use `dolt cherry-pick --abort`.\n" + + "Run `dolt sql -q 'select * from dolt_test_run()` to see which tests are failing.") + type CherryPickCmd struct{} // Name returns the name of the Dolt cli command. This is what is used on the command line to invoke the command. @@ -166,6 +173,10 @@ hint: commit your changes (dolt commit -am \"\") or reset them (dolt re } rows, err := cli.GetRowsForSql(queryist, sqlCtx, q) if err != nil { + if actions.ErrCommitVerificationFailed.Is(err) { + cli.PrintErrln(err.Error()) + return ErrCherryPickVerificationFailed.New() + } errorText := err.Error() switch { case strings.Contains("nothing to commit", errorText): @@ -250,20 +261,27 @@ func cherryPickContinue(sqlCtx *sql.Context, queryist cli.Queryist) error { query := "call dolt_cherry_pick('--continue')" rows, err := cli.GetRowsForSql(queryist, sqlCtx, query) if err != nil { + if actions.ErrCommitVerificationFailed.Is(err) { + cli.PrintErrln(err.Error()) + return ErrCherryPickVerificationFailed.New() + } return err } if len(rows) != 1 { return fmt.Errorf("error: unexpected number of rows returned from dolt_cherry_pick: %d", len(rows)) } - if len(rows[0]) != 4 { + + // No version of dolt_cherry_pick has ever returned less than 4 columns. We don't set an upper bound here to + // allow for servers to add more columns in the future without breaking compatibility with older clients. + if len(rows[0]) < 4 { return fmt.Errorf("error: unexpected number of columns returned from dolt_cherry_pick: %d", len(rows[0])) } row := rows[0] // We expect to get an error if there were problems, but we also could get any of the conflicts and - // vacation counts being greater than 0 if there were problems. If we got here without an error, + // violation counts being greater than 0 if there were problems. If we got here without an error, // but we have conflicts or violations, we should report and stop. dataConflicts, err := getInt64ColAsInt64(row[1]) if err != nil { diff --git a/go/cmd/dolt/commands/merge.go b/go/cmd/dolt/commands/merge.go index db5aea5ab02..9c65f8c3c5c 100644 --- a/go/cmd/dolt/commands/merge.go +++ b/go/cmd/dolt/commands/merge.go @@ -33,6 +33,7 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/diff" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/env" + "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" "github.com/dolthub/dolt/go/libraries/doltcore/merge" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dprocedures" "github.com/dolthub/dolt/go/libraries/utils/argparser" @@ -165,6 +166,11 @@ func (cmd MergeCmd) Exec(ctx context.Context, commandStr string, args []string, return 1 } + if msg := getMergeMessage(mergeResultRow); strings.HasPrefix(msg, actions.CommitVerificationFailedPrefix) { + cli.Println(msg) + return 1 + } + if !apr.Contains(cli.AbortParam) { //todo: refs with the `remotes/` prefix will fail to get a hash headHash, headHashErr := getHashOf(queryist.Queryist, queryist.Context, "HEAD") @@ -751,3 +757,17 @@ func everythingUpToDate(row sql.Row) (bool, error) { return false, nil } + +// getMergeMessage extracts the message column from a merge result row. +func getMergeMessage(row sql.Row) string { + if len(row) == 3 { + if msg, ok := row[2].(string); ok { + return msg + } + } else if len(row) == 4 { + if msg, ok := row[3].(string); ok { + return msg + } + } + return "" +} diff --git a/go/cmd/dolt/commands/rebase.go b/go/cmd/dolt/commands/rebase.go index ea8ec288a1e..cce2010b26c 100644 --- a/go/cmd/dolt/commands/rebase.go +++ b/go/cmd/dolt/commands/rebase.go @@ -28,6 +28,7 @@ import ( "github.com/dolthub/dolt/go/cmd/dolt/errhand" "github.com/dolthub/dolt/go/libraries/doltcore/dconfig" "github.com/dolthub/dolt/go/libraries/doltcore/env" + "github.com/dolthub/dolt/go/libraries/doltcore/env/actions" "github.com/dolthub/dolt/go/libraries/doltcore/rebase" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dprocedures" "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess" @@ -429,7 +430,8 @@ func syncCliBranchToSqlSessionBranch(ctx *sql.Context, dEnv *env.DoltEnv) error } // isRebaseConflictError checks if the given error represents a rebase pause condition -// (data conflicts) that should not abort the rebase but instead allow the user to resolve/continue. +// (data conflicts or verification failure) that should not abort the rebase but instead +// allow the user to resolve/continue. func isRebaseConflictError(err error) bool { if err == nil { return false @@ -439,8 +441,16 @@ func isRebaseConflictError(err error) bool { if dprocedures.ErrRebaseDataConflict.Is(err) { return true } + if dprocedures.ErrRebaseVerificationFailed.Is(err) { + return true + } + if actions.ErrCommitVerificationFailed.Is(err) { + return true + } // For over-the-wire errors that lose their type, match against error message patterns errMsg := err.Error() - return strings.HasPrefix(errMsg, dprocedures.RebaseDataConflictPrefix) + return strings.HasPrefix(errMsg, dprocedures.RebaseDataConflictPrefix) || + strings.HasPrefix(errMsg, dprocedures.RebaseVerificationFailedPrefix) || + strings.HasPrefix(errMsg, actions.CommitVerificationFailedPrefix) } diff --git a/go/libraries/doltcore/cherry_pick/cherry_pick.go b/go/libraries/doltcore/cherry_pick/cherry_pick.go index b7172dc3819..95ea09f3cd0 100644 --- a/go/libraries/doltcore/cherry_pick/cherry_pick.go +++ b/go/libraries/doltcore/cherry_pick/cherry_pick.go @@ -82,6 +82,14 @@ func CherryPick(ctx *sql.Context, commit string, options CherryPickOptions) (str return "", nil, fmt.Errorf("failed to get roots for current session") } + // Capture the working set BEFORE applying cherry-pick changes. If commit verification + // fails later, we need to set the merge state with preMergeWorking = original working + // root so that --abort properly restores the pre-cherry-pick state. + preApplyWs, wsErr := doltSession.WorkingSet(ctx, dbName) + if wsErr != nil { + return "", nil, wsErr + } + mergeResult, commitMsg, originalCommit, err := cherryPick(ctx, doltSession, roots, dbName, commit, options.EmptyCommitHandling) if err != nil { return "", mergeResult, err @@ -131,6 +139,20 @@ func CherryPick(ctx *sql.Context, commit string, options CherryPickOptions) (str pendingCommit, err := doltSession.NewPendingCommit(ctx, dbName, roots, *commitProps) if err != nil { + // If verification failed, set merge state and return via the result (not as a Go error). + // Returning a Go error would roll back the transaction, undoing all working set changes. + // Returning nil error instead lets CommitTransaction persist the staged changes and merge + // state, so the user can fix the failing tests and --continue. + if actions.ErrCommitVerificationFailed.Is(err) { + newWs := preApplyWs.StartCherryPick(originalCommit, commit). + WithWorkingRoot(roots.Working). + WithStagedRoot(roots.Staged) + if wsErr := doltSession.SetWorkingSet(ctx, dbName, newWs); wsErr != nil { + return "", nil, wsErr + } + mergeResult.CommitVerificationErr = err + return "", mergeResult, nil + } return "", nil, err } if pendingCommit == nil { @@ -231,9 +253,10 @@ func AbortCherryPick(ctx *sql.Context, dbName string) error { return doltSession.SetWorkingSet(ctx, dbName, newWs) } -// ContinueCherryPick continues a cherry-pick merge that was paused due to conflicts. -// It checks that conflicts have been resolved and creates the final commit with the -// original commit's metadata. +// ContinueCherryPick continues a cherry-pick merge that was paused due to conflicts or +// commit verification failure. It checks that conflicts have been resolved and creates +// the final commit with the original commit's metadata. Returns (hash, dataConflicts, +// schemaConflicts, constraintViolations, verificationFailures, error). func ContinueCherryPick(ctx *sql.Context, dbName string) (string, int, int, int, error) { doltSession := dsess.DSessFromSess(ctx.Session) @@ -310,6 +333,9 @@ func ContinueCherryPick(ctx *sql.Context, dbName string) (string, int, int, int, pendingCommit, err := doltSession.NewPendingCommit(ctx, dbName, roots, commitProps) if err != nil { + if actions.ErrCommitVerificationFailed.Is(err) { + return "", 0, 0, 0, err + } return "", 0, 0, 0, fmt.Errorf("error: failed to create pending commit: %w", err) } if pendingCommit == nil { diff --git a/go/libraries/doltcore/env/actions/commit.go b/go/libraries/doltcore/env/actions/commit.go index 15b1a619e2d..c85ee4497e2 100644 --- a/go/libraries/doltcore/env/actions/commit.go +++ b/go/libraries/doltcore/env/actions/commit.go @@ -22,12 +22,17 @@ import ( gms "github.com/dolthub/go-mysql-server" "github.com/dolthub/go-mysql-server/sql" + goerrors "gopkg.in/src-d/go-errors.v1" "github.com/dolthub/dolt/go/libraries/doltcore/diff" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/store/datas" ) +const CommitVerificationFailedPrefix = "commit verification failed:" + +var ErrCommitVerificationFailed = goerrors.NewKind(CommitVerificationFailedPrefix + " %s") + type CommitStagedProps struct { Message string Date time.Time @@ -45,10 +50,10 @@ const ( DoltCommitVerificationGroups = "dolt_commit_verification_groups" ) -// GetCommitRunTestGroups returns the test groups to run for commit operations +// getCommitRunTestGroups returns the test groups to run for commit operations // Returns empty slice if no tests should be run, ["*"] if all tests should be run, // or specific group names if only those groups should be run -func GetCommitRunTestGroups() []string { +func getCommitRunTestGroups() []string { _, val, ok := sql.SystemVariables.GetGlobal(DoltCommitVerificationGroups) if !ok { return nil @@ -147,7 +152,7 @@ func GetCommitStaged( } if !props.SkipVerification { - testGroups := GetCommitRunTestGroups() + testGroups := getCommitRunTestGroups() if len(testGroups) > 0 { err := runCommitVerification(ctx, testGroups) if err != nil { @@ -164,6 +169,9 @@ func GetCommitStaged( return db.NewPendingCommit(ctx, roots, mergeParents, props.Amend, meta) } +// runCommitVerification runs the commit verification tests for the given test groups. +// If any tests fail, it returns ErrCommitVerificationFailed wrapping the failure details. +// Callers can use errors.Is(err, ErrCommitVerificationFailed) to detect this case. func runCommitVerification(ctx *sql.Context, testGroups []string) error { type sessionInterface interface { sql.Session @@ -216,7 +224,7 @@ func runTestsUsingDtablefunctions(ctx *sql.Context, engine *gms.Engine, testGrou } if len(allFailures) > 0 { - return fmt.Errorf("commit verification failed: %s", strings.Join(allFailures, ", ")) + return ErrCommitVerificationFailed.New(strings.Join(allFailures, ", ")) } return nil diff --git a/go/libraries/doltcore/merge/merge.go b/go/libraries/doltcore/merge/merge.go index ec84f0f3e66..c8f066f47b2 100644 --- a/go/libraries/doltcore/merge/merge.go +++ b/go/libraries/doltcore/merge/merge.go @@ -87,9 +87,10 @@ func MergeCommits(ctx *sql.Context, tableResolver doltdb.TableResolver, commit, } type Result struct { - Root doltdb.RootValue - SchemaConflicts []SchemaConflict - Stats map[doltdb.TableName]*MergeStats + Root doltdb.RootValue + SchemaConflicts []SchemaConflict + Stats map[doltdb.TableName]*MergeStats + CommitVerificationErr error } func (r Result) HasSchemaConflicts() bool { diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go b/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go index ce6833490a0..3623a193e7f 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_cherry_pick.go @@ -25,6 +25,7 @@ import ( "github.com/dolthub/dolt/go/libraries/doltcore/branch_control" "github.com/dolthub/dolt/go/libraries/doltcore/cherry_pick" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/doltcore/sqle/dsess" ) var ErrEmptyCherryPick = errors.New("cannot cherry-pick empty string") @@ -64,6 +65,7 @@ func doltCherryPick(ctx *sql.Context, args ...string) (sql.RowIter, error) { // doDoltCherryPick attempts to perform a cherry-pick merge based on the arguments specified in |args| and returns // the new, created commit hash (if it was successful created), a count of the number of tables with data conflicts, // a count of the number of tables with schema conflicts, and a count of the number of tables with constraint violations. +// Verification failures are returned as errors. func doDoltCherryPick(ctx *sql.Context, args []string) (string, int, int, int, error) { // Get the information for the sql context. dbName := ctx.GetCurrentDatabase() @@ -119,6 +121,15 @@ func doDoltCherryPick(ctx *sql.Context, args []string) (string, int, int, int, e } if mergeResult != nil { + if mergeResult.CommitVerificationErr != nil { + // Commit the transaction to persist the dirty working set to the staged working set. + // This allows the user to address the verification failure and then `--continue` the cherry-pick. + doltSession := dsess.DSessFromSess(ctx.Session) + if txErr := doltSession.CommitTransaction(ctx, doltSession.GetTransaction()); txErr != nil { + return "", 0, 0, 0, txErr + } + return "", 0, 0, 0, mergeResult.CommitVerificationErr + } return "", mergeResult.CountOfTablesWithDataConflicts(), mergeResult.CountOfTablesWithSchemaConflicts(), diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go index 1f68de8e792..083d8df48ff 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_merge.go @@ -245,6 +245,10 @@ func performMerge( } ctx.Warn(DoltMergeWarningCode, "%s", err.Error()) return ws, "", hasConflictsOrViolations, threeWayMerge, "", err + } else if actions.ErrCommitVerificationFailed.Is(err) { + // We don't return an error here because that rolls back the transaction, while we actually need the + // staged data to allow the user to address the verification problem. + return ws, "", noConflictsOrViolations, threeWayMerge, err.Error(), nil } else if err != nil { return ws, "", noConflictsOrViolations, threeWayMerge, "", err } @@ -314,6 +318,9 @@ func performMerge( } commit, _, err = doDoltCommit(ctx, args) if err != nil { + if actions.ErrCommitVerificationFailed.Is(err) { + return ws, "", noConflictsOrViolations, threeWayMerge, err.Error(), nil + } return ws, commit, noConflictsOrViolations, threeWayMerge, "", err } } @@ -457,6 +464,9 @@ func executeNoFFMerge( SkipVerification: skipVerification, }) if err != nil { + if actions.ErrCommitVerificationFailed.Is(err) { + return ws, nil, err + } return nil, nil, err } diff --git a/go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go b/go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go index a770f6485f9..7d665f02402 100644 --- a/go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go +++ b/go/libraries/doltcore/sqle/dprocedures/dolt_rebase.go @@ -144,6 +144,13 @@ var ErrRebaseDataConflict = goerrors.NewKind(RebaseDataConflictPrefix + " %s (%s "Resolve the conflicts and remove them from the dolt_conflicts_ tables, " + "then continue the rebase by calling dolt_rebase('--continue')") +const RebaseVerificationFailedPrefix = "commit verification failed while rebasing commit" + +// ErrRebaseVerificationFailed is used when commit verification tests fail while rebasing a commit. +// The workspace is left dirty so the user can fix the failing tests and retry using --continue. +var ErrRebaseVerificationFailed = goerrors.NewKind(RebaseVerificationFailedPrefix + " %s (%s): %s\n\n" + + "Fix the failing tests and stage your changes, then continue the rebase by calling dolt_rebase('--continue')") + var EditPausePrefix = "edit action paused at commit" // createEditPauseMessage creates a pause message for edit actions @@ -985,6 +992,19 @@ func handleRebaseCherryPick( return newRebaseError(ErrRebaseDataConflict.New(planStep.CommitHash, planStep.CommitMsg)) } + if mergeResult != nil && mergeResult.CommitVerificationErr != nil { + if doltSession.GetTransaction() == nil { + _, txErr := doltSession.StartTransaction(ctx, sql.ReadWrite) + if txErr != nil { + return newRebaseError(txErr) + } + } + if txErr := doltSession.CommitTransaction(ctx, doltSession.GetTransaction()); txErr != nil { + return newRebaseError(txErr) + } + return newRebaseError(mergeResult.CommitVerificationErr) + } + // If this is an edit action and no conflicts occurred, pause the rebase to allow user modifications if planStep.Action == rebase.RebaseActionEdit && err == nil { return newRebasePause(createEditPauseMessage(planStep.CommitHash, planStep.CommitMsg)) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_commit_verification.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_commit_verification.go index 0a2251cc953..08eb75e1753 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_commit_verification.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_commit_verification.go @@ -188,7 +188,7 @@ var DoltCommitVerificationScripts = []queries.ScriptTest{ }, }, { - Name: "cherry-pick with test verification enabled - tests pass", + Name: "cherry-pick with test verification enabled - test pass, then test fails", SetUpScript: []string{ "SET GLOBAL dolt_commit_verification_groups = '*'", "CREATE TABLE users (id INT PRIMARY KEY, name VARCHAR(100) NOT NULL, email VARCHAR(100))", @@ -209,6 +209,7 @@ var DoltCommitVerificationScripts = []queries.ScriptTest{ }, Assertions: []queries.ScriptTestAssertion{ { + // Verify that tests _can_ pass by cherry-picking the first commit, which adds Bob and updates the test to expect 2 users. Query: "CALL dolt_cherry_pick(@commit_1_hash)", Expected: []sql.Row{{commitHash, int64(0), int64(0), int64(0)}}, }, @@ -216,6 +217,21 @@ var DoltCommitVerificationScripts = []queries.ScriptTest{ Query: "CALL dolt_cherry_pick(@commit_2_hash)", ExpectedErrStr: "commit verification failed: test_user_count_update (Assertion failed: expected_single_value equal to 2, got 3)", }, + { + // users table is staged (merged content with charlie added is preserved) + Query: "SELECT staged FROM dolt_status WHERE table_name = 'users'", + Expected: []sql.Row{{uint64(1)}}, + }, + { + // --abort restores clean state, with Bob in the table (from first cherry-pick). + Query: "CALL dolt_cherry_pick('--abort')", + SkipResultsCheck: true, + }, + { + Query: "SELECT COUNT(*) FROM users", + Expected: []sql.Row{{int64(2)}}, + }, + {}, { // Test harness bleeds GLOBAL variable changes across tests, so reset after each test. Query: "SET GLOBAL dolt_commit_verification_groups = ''", SkipResultsCheck: true, @@ -223,7 +239,7 @@ var DoltCommitVerificationScripts = []queries.ScriptTest{ }, }, { - Name: "cherry-pick with test verification enabled - tests fail, aborted", + Name: "cherry-pick with test verification enabled - tests fail, leaves dirty state", SetUpScript: []string{ "SET GLOBAL dolt_commit_verification_groups = '*'", "CREATE TABLE users (id INT PRIMARY KEY, name VARCHAR(100) NOT NULL, email VARCHAR(100))", @@ -240,18 +256,86 @@ var DoltCommitVerificationScripts = []queries.ScriptTest{ }, Assertions: []queries.ScriptTestAssertion{ { + // Verification fails; returns specific error, dirty state preserved via explicit CommitTransaction Query: "CALL dolt_cherry_pick(@commit_hash)", ExpectedErrStr: "commit verification failed: test_users_count (Assertion failed: expected_single_value equal to 1, got 2)", }, { + // users table should be staged (cherry-pick changes are preserved by the committed transaction) + Query: "SELECT staged FROM dolt_status WHERE table_name = 'users'", + Expected: []sql.Row{{uint64(1)}}, + }, + { + // --continue without fixing still fails with specific error, dirty state preserved + Query: "CALL dolt_cherry_pick('--continue')", + ExpectedErrStr: "commit verification failed: test_users_count (Assertion failed: expected_single_value equal to 1, got 2)", + }, + { + // Abort restores clean state + Query: "CALL dolt_cherry_pick('--abort')", + SkipResultsCheck: true, + }, + { + // After abort, workspace is clean + Query: "SELECT COUNT(*) FROM dolt_status", + Expected: []sql.Row{{int64(0)}}, + }, + { + // Now cherry-pick succeeds with --skip-verification Query: "CALL dolt_cherry_pick('--skip-verification', @commit_hash)", Expected: []sql.Row{{commitHash, int64(0), int64(0), int64(0)}}, }, + { // Test harness bleeds GLOBAL variable changes across tests, so reset after each test. + Query: "SET GLOBAL dolt_commit_verification_groups = ''", + SkipResultsCheck: true, + }, + }, + }, + { + Name: "cherry-pick with test verification enabled - fix data and --continue succeeds", + SetUpScript: []string{ + "SET GLOBAL dolt_commit_verification_groups = '*'", + "CREATE TABLE users (id INT PRIMARY KEY, name VARCHAR(100) NOT NULL, email VARCHAR(100))", + "INSERT INTO users VALUES (1, 'Alice', 'alice@example.com')", + "INSERT INTO dolt_tests (test_name, test_group, test_query, assertion_type, assertion_comparator, assertion_value) VALUES " + + "('test_users_count', 'unit', 'SELECT COUNT(*) FROM users', 'expected_single_value', '==', '1')", + "CALL dolt_add('.')", + "CALL dolt_commit('-m', 'Initial commit')", + "CALL dolt_checkout('-b', 'feature')", + "INSERT INTO users VALUES (2, 'Bob', 'bob@example.com')", + "CALL dolt_add('.')", + "call dolt_commit_hash_out(@commit_hash,'--skip-verification', '-m', 'Add Bob but dont update test')", + "CALL dolt_checkout('main')", + }, + Assertions: []queries.ScriptTestAssertion{ + { + // Verification fails; returns specific error, dirty state preserved via explicit CommitTransaction + Query: "CALL dolt_cherry_pick(@commit_hash)", + ExpectedErrStr: "commit verification failed: test_users_count (Assertion failed: expected_single_value equal to 1, got 2)", + }, { - Query: "select * from dolt_test_run('*')", - Expected: []sql.Row{ - {"test_users_count", "unit", "SELECT COUNT(*) FROM users", "FAIL", "Assertion failed: expected_single_value equal to 1, got 2"}, - }, + // Fix the test to match the new row count and stage + Query: "UPDATE dolt_tests SET assertion_value = '2' WHERE test_name = 'test_users_count'", + SkipResultsCheck: true, + }, + { + Query: "CALL dolt_add('.')", + SkipResultsCheck: true, + }, + { + // --continue now passes verification and creates the commit + Query: "CALL dolt_cherry_pick('--continue')", + Expected: []sql.Row{{commitHash, int64(0), int64(0), int64(0)}}, + }, + { + // Workspace is clean after successful --continue + Query: "SELECT COUNT(*) FROM dolt_status", + Expected: []sql.Row{{int64(0)}}, + }, + { + // Both users are present + Query: "SELECT COUNT(*) FROM users", + Expected: []sql.Row{{int64(2)}}, }, { // Test harness bleeds GLOBAL variable changes across tests, so reset after each test. Query: "SET GLOBAL dolt_commit_verification_groups = ''", @@ -470,7 +554,7 @@ var DoltCommitVerificationScripts = []queries.ScriptTest{ }, }, { - Name: "merge with test verification enabled - tests fail, merge aborted", + Name: "merge with test verification enabled - tests fail, leaves dirty state", SetUpScript: []string{ "SET GLOBAL dolt_commit_verification_groups = '*'", "CREATE TABLE users (id INT PRIMARY KEY, name VARCHAR(100) NOT NULL, email VARCHAR(100))", @@ -490,8 +574,81 @@ var DoltCommitVerificationScripts = []queries.ScriptTest{ }, Assertions: []queries.ScriptTestAssertion{ { - Query: "CALL dolt_merge('feature')", - ExpectedErrStr: "commit verification failed: test_will_fail (Assertion failed: expected_single_value equal to 999, got 3)", + // Verification fails; returns verification failure in message field with no SQL error + // so dirty merge state is preserved (transaction commits). + Query: "CALL dolt_merge('feature')", + Expected: []sql.Row{{"", int64(0), int64(0), "commit verification failed: test_will_fail (Assertion failed: expected_single_value equal to 999, got 3)"}}, + }, + { + // users table is staged (merged content with Bob added is preserved) + Query: "SELECT staged FROM dolt_status WHERE table_name = 'users'", + Expected: []sql.Row{{uint64(1)}}, + }, + { // Test harness bleeds GLOBAL variable changes across tests, so reset after each test. + Query: "SET GLOBAL dolt_commit_verification_groups = ''", + SkipResultsCheck: true, + }, + }, + }, + { + Name: "merge with test verification enabled - fix data and dolt_commit succeeds", + SetUpScript: []string{ + "SET GLOBAL dolt_commit_verification_groups = '*'", + "CREATE TABLE users (id INT PRIMARY KEY, name VARCHAR(100) NOT NULL, email VARCHAR(100))", + "INSERT INTO users VALUES (1, 'Alice', 'alice@example.com')", + "INSERT INTO dolt_tests (test_name, test_group, test_query, assertion_type, assertion_comparator, assertion_value) VALUES " + + "('test_users_count', 'unit', 'SELECT COUNT(*) FROM users', 'expected_single_value', '==', '1')", + "CALL dolt_add('.')", + "CALL dolt_commit('-m', 'Initial commit')", + "CALL dolt_checkout('-b', 'feature')", + "INSERT INTO users VALUES (2, 'Bob', 'bob@example.com')", + "CALL dolt_add('.')", + "CALL dolt_commit('--skip-verification', '-m', 'Add Bob but dont update test')", + "CALL dolt_checkout('main')", + "INSERT INTO users VALUES (3, 'Charlie', 'charlie@example.com')", + "CALL dolt_add('.')", + "CALL dolt_commit('--skip-verification', '-m', 'Add Charlie to force non-FF merge')", + }, + Assertions: []queries.ScriptTestAssertion{ + { + // Verification fails; returns verification failure in message field with no SQL error + // so dirty merge state is preserved (transaction commits). + Query: "CALL dolt_merge('feature')", + Expected: []sql.Row{{"", int64(0), int64(0), "commit verification failed: test_users_count (Assertion failed: expected_single_value equal to 1, got 3)"}}, + }, + { + // dolt_commit without fixing still fails with verification error; dirty state still preserved + Query: "CALL dolt_commit('-m', 'Complete merge without fixing')", + ExpectedErrStr: "commit verification failed: test_users_count (Assertion failed: expected_single_value equal to 1, got 3)", + }, + { + // users table is still staged (dirty state was not lost by the failed dolt_commit) + Query: "SELECT staged FROM dolt_status WHERE table_name = 'users'", + Expected: []sql.Row{{uint64(1)}}, + }, + { + // Fix the test expectation to match the merged user count + Query: "UPDATE dolt_tests SET assertion_value = '3' WHERE test_name = 'test_users_count'", + SkipResultsCheck: true, + }, + { + Query: "CALL dolt_add('.')", + SkipResultsCheck: true, + }, + { + // Now dolt_commit succeeds and creates a merge commit + Query: "CALL dolt_commit('-m', 'Complete merge after fixing test')", + Expected: []sql.Row{{commitHash}}, + }, + { + // Workspace is clean after successful merge commit + Query: "SELECT COUNT(*) FROM dolt_status", + Expected: []sql.Row{{int64(0)}}, + }, + { + // All three users are present after merge + Query: "SELECT COUNT(*) FROM users", + Expected: []sql.Row{{int64(3)}}, }, { // Test harness bleeds GLOBAL variable changes across tests, so reset after each test. Query: "SET GLOBAL dolt_commit_verification_groups = ''", diff --git a/integration-tests/bats/cherry-pick.bats b/integration-tests/bats/cherry-pick.bats index 6dd8a26e483..79b3d3c4c1e 100644 --- a/integration-tests/bats/cherry-pick.bats +++ b/integration-tests/bats/cherry-pick.bats @@ -730,4 +730,4 @@ teardown() { [ $status -eq 1 ] [[ $output =~ "Unable to apply commit cleanly due to conflicts" ]] || false -} \ No newline at end of file +} diff --git a/integration-tests/bats/commit_verification.bats b/integration-tests/bats/commit_verification.bats index 7a0d9b7b861..c76be36ef84 100644 --- a/integration-tests/bats/commit_verification.bats +++ b/integration-tests/bats/commit_verification.bats @@ -107,33 +107,37 @@ SQL [ "$status" -eq 0 ] } -@test "commit_verification: merge with tests enabled - tests fail, merge aborted" { +@test "commit_verification: merge with tests enabled - tests fail, can abort and retry with --skip-verification" { dolt sql -q "SET @@PERSIST.dolt_commit_verification_groups = '*'" - + dolt sql <