[release-23.0] vttestserver: fail, don't panic, on a vschema missing a table or vindex (#20740) by vitess-bot[bot] · Pull Request #20744 · vitessio/vitess

vitess-bot · 2026-07-29T12:44:52Z

Description

This is a backport of #20740

vitess-bot · 2026-07-29T12:44:55Z

Hello @arthurschreiber, there are conflicts in this backport.

Please address them in order to merge this Pull Request. You can execute the snippet below to reset your branch and resolve the conflict manually.

Make sure you replace origin by the name of the vitessio/vitess remote

git fetch --all
gh pr checkout 20744
git reset --hard origin/release-23.0
git cherry-pick -m 1 bf2a56ab9e40724eae9f239965b6a0357d3c854b

Copilot

Pull request overview

Note

Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.

This PR updates the vttestserver CLI tests to avoid panics when a vschema is missing expected entries, aiming to fail with clearer, assertion-based error messages instead.

Changes:

Adds safer vschema lookups (table/vindex) prior to dereferencing to prevent nil-map-entry panics.
Switches JSON unmarshal handling to require.*-style assertions for immediate test failure.
Updates imports to support sorted/stable debug output for missing vschema keys.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

 	"context"
 	"fmt"
 	"io"
+	"maps"
 	"math/rand/v2"
 	"os/exec"
 	"path"
+<<<<<<< HEAD
+||||||| parent of bf2a56ab9e (vttestserver: fail, don't panic, on a vschema missing a table or vindex (#20740))
+	"strconv"
+=======
+	"slices"
+	"strconv"
+>>>>>>> bf2a56ab9e (vttestserver: fail, don't panic, on a vschema missing a table or vindex (#20740))
 	"strings"


 	err := vtctlclient.RunCommandAndWait(ctx, server, args, func(e *logutilpb.Event) {
 		var keyspace vschemapb.Keyspace
+<<<<<<< HEAD
 		if err := protojson.Unmarshal([]byte(e.Value), &keyspace); err != nil {
 			t.Error(err)
 		}
+||||||| parent of bf2a56ab9e (vttestserver: fail, don't panic, on a vschema missing a table or vindex (#20740))
+		if err := protojson.Unmarshal([]byte(e.Value), &keyspace); err != nil {
+			assert.NoError(t, err)
+		}
+=======
+		require.NoError(t, protojson.Unmarshal([]byte(e.Value), &keyspace))
+
+		// Look the table and vindex up before dereferencing them, so that a
+		// vschema missing either fails with a message naming what was missing
+		// instead of panicking on a nil map entry and taking down the whole
+		// test binary.
+		table, ok := keyspace.Tables[expected.table]
+		require.Truef(t, ok, "keyspace %s has no table %s in its vschema, found tables %v", expected.keyspace, expected.table, slices.Sorted(maps.Keys(keyspace.Tables)))
+		require.NotEmptyf(t, table.ColumnVindexes, "table %s.%s has no column vindexes", expected.keyspace, expected.table)
+		columnVindex := table.ColumnVindexes[0]
+		require.NotEmptyf(t, columnVindex.Columns, "column vindex %s on %s.%s has no columns", columnVindex.Name, expected.keyspace, expected.table)
+
+		actualVindex, ok := keyspace.Vindexes[expected.vindex]
+		require.Truef(t, ok, "keyspace %s has no vindex %s in its vschema, found vindexes %v", expected.keyspace, expected.vindex, slices.Sorted(maps.Keys(keyspace.Vindexes)))
+>>>>>>> bf2a56ab9e (vttestserver: fail, don't panic, on a vschema missing a table or vindex (#20740))


Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

arthurschreiber · 2026-07-29T14:51:17Z

Resolved the committed conflicts in go/cmd/vttestserver/cli/main_test.go (new commit, no history rewrite). Two hunks: the assertColumnVindex body (release branch had t.Error(err) where main had assert.NoError) — resolved by taking the upstream #20740 version — and the import block, where release-23.0 doesn't import strconv, so only the new slices import was added. The diff against release-23.0 now matches the upstream PR exactly.

arthurschreiber · 2026-07-29T16:34:37Z

CI note: the Upgrade Downgrade Test - Backups - E2E failure (vttablet=N-1, vtbackup=N step, ResetReplication ... DEADLINE_EXCEEDED during TestTabletBackupOnly) is not caused by this PR — the same step fails identically on every recent release-23.0 backport PR (e.g. #20722, #20402 branches), and this PR only touches a vttestserver test file. The check is not in the branch's required status checks. All other checks are green (106/107).

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4800f9d376

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-29T16:37:34Z

+		table, ok := keyspace.Tables[expected.table]
+		require.Truef(t, ok, "keyspace %s has no table %s in its vschema, found tables %v", expected.keyspace, expected.table, slices.Sorted(maps.Keys(keyspace.Tables)))


Add a regression test that reaches the missing-entry guards

Every caller still requests tables and vindexes present in the fixture, so these new branches are never exercised and the suite passes identically if the guards are reverted to the panicking implementation. Add a focused test that supplies a missing table and vindex and verifies that validation reports a test failure rather than panicking; otherwise CI does not prove the behavior this commit claims to fix.

AGENTS.md reference: AGENTS.md:L72-L73

Useful? React with 👍 / 👎.

github-actions · 2026-08-29T01:15:02Z

This PR is being marked as stale because it has been open for 30 days with no activity. To rectify, you may do any of the following:

Push additional commits to the associated branch.
Remove the stale label.
Add a comment indicating why it is not stale.

If no action is taken within 7 days, this PR will be closed.

Cherry-pick bf2a56a with conflicts

6292d95

Copilot AI review requested due to automatic review settings July 29, 2026 12:44

vitess-bot Bot added Type: Testing Component: vttestserver Backport This is a backport Skip CI Skip CI actions from running Merge Conflict labels Jul 29, 2026

vitess-bot Bot requested a review from arthurschreiber July 29, 2026 12:44

github-actions Bot added this to the v23.0.6 milestone Jul 29, 2026

Copilot AI reviewed Jul 29, 2026

View reviewed changes

arthurschreiber self-assigned this Jul 29, 2026

resolve conflicts for backport of #20740

4800f9d

Signed-off-by: Arthur Schreiber <arthur@planetscale.com>

arthurschreiber removed Skip CI Skip CI actions from running Merge Conflict labels Jul 29, 2026

Copilot AI review requested due to automatic review settings July 29, 2026 14:50

Copilot AI reviewed Jul 29, 2026

arthurschreiber marked this pull request as ready for review July 29, 2026 16:34

arthurschreiber requested a review from mattlord as a code owner July 29, 2026 16:34

arthurschreiber enabled auto-merge (squash) July 29, 2026 16:34

chatgpt-codex-connector Bot reviewed Jul 29, 2026

View reviewed changes

github-actions Bot added the Stale Marks PRs as stale after a period of inactivity, which are then closed after a grace period. label Aug 29, 2026

Navigation Menu

Uh oh!

[release-23.0] vttestserver: fail, don't panic, on a vschema missing a table or vindex (#20740) - #20744

[release-23.0] vttestserver: fail, don't panic, on a vschema missing a table or vindex (#20740)#20744
vitess-bot[bot] wants to merge 2 commits into
release-23.0from
backport-20740-to-release-23.0

vitess-bot Bot commented Jul 29, 2026

Uh oh!

vitess-bot Bot commented Jul 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

arthurschreiber commented Jul 29, 2026

Uh oh!

arthurschreiber commented Jul 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 29, 2026

Uh oh!

github-actions Bot commented Aug 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		table, ok := keyspace.Tables[expected.table]
		require.Truef(t, ok, "keyspace %s has no table %s in its vschema, found tables %v", expected.keyspace, expected.table, slices.Sorted(maps.Keys(keyspace.Tables)))

Uh oh!

Conversation

vitess-bot Bot commented Jul 29, 2026

Description

Uh oh!

vitess-bot Bot commented Jul 29, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

arthurschreiber commented Jul 29, 2026

Uh oh!

arthurschreiber commented Jul 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants