laskoviymishka commented on code in PR #1071:
URL: https://github.com/apache/iceberg-go/pull/1071#discussion_r3236917684
##########
cmd/iceberg/upgrade_rollback.go:
##########
@@ -19,23 +19,144 @@ package main
import (
"context"
- "errors"
+ "encoding/json"
+ "fmt"
"os"
+ "strconv"
"github.com/apache/iceberg-go/catalog"
+ "github.com/apache/iceberg-go/table"
+ "github.com/pterm/pterm"
)
-func runUpgrade(_ context.Context, output Output, _ catalog.Catalog, _
*UpgradeCmd) {
- output.Error(errors.New("upgrade: not yet implemented"))
- os.Exit(1)
+func runUpgrade(ctx context.Context, output Output, cat catalog.Catalog, cmd
*UpgradeCmd) {
+ tbl := loadTable(ctx, output, cat, cmd.TableID)
+ meta := tbl.Metadata()
+ currentVersion := meta.Version()
+
+ if cmd.FormatVersion <= currentVersion {
+ output.Error(fmt.Errorf("target format version %d must be
greater than current version %d",
+ cmd.FormatVersion, currentVersion))
+ os.Exit(1)
+ }
+
+ result := UpgradeResult{
+ DryRun: cmd.DryRun,
+ Table: tableIDString(tbl),
+ PreviousVersion: currentVersion,
+ TargetVersion: cmd.FormatVersion,
+ SpecURL: specURL(cmd.FormatVersion),
+ }
+
+ if cmd.DryRun {
+ output.UpgradeResult(result)
+
+ return
+ }
+
+ prompt := fmt.Sprintf("Upgrade %s from format version %d to %d?",
+ tableIDString(tbl), currentVersion, cmd.FormatVersion)
+ if err := confirmAction(prompt, cmd.Yes); err != nil {
+ output.Error(err)
+ os.Exit(1)
+ }
+
+ tx := tbl.NewTransaction()
+ if err := tx.UpgradeFormatVersion(cmd.FormatVersion); err != nil {
+ output.Error(fmt.Errorf("upgrade failed: %w", err))
+ os.Exit(1)
+ }
+
+ if _, err := tx.Commit(ctx); err != nil {
+ output.Error(fmt.Errorf("commit failed: %w", err))
+ os.Exit(1)
+ }
+
+ output.UpgradeResult(result)
+}
+
+func runRollback(ctx context.Context, output Output, cat catalog.Catalog, cmd
*RollbackCmd) {
+ tbl := loadTable(ctx, output, cat, cmd.TableID)
+ meta := tbl.Metadata()
+
+ snap := meta.SnapshotByID(cmd.SnapshotID)
+ if snap == nil {
+ output.Error(fmt.Errorf("snapshot %d not found in table %s",
cmd.SnapshotID, tableIDString(tbl)))
+ os.Exit(1)
+ }
+
+ var previousSnapshotID *int64
+ if cs := meta.CurrentSnapshot(); cs != nil {
+ id := cs.SnapshotID
+ previousSnapshotID = &id
+ }
+
+ prompt := fmt.Sprintf("Roll back %s to snapshot %d?",
tableIDString(tbl), cmd.SnapshotID)
+ if err := confirmAction(prompt, cmd.Yes); err != nil {
+ output.Error(err)
+ os.Exit(1)
+ }
+
+ update := table.NewSetSnapshotRefUpdate(table.MainBranch,
cmd.SnapshotID, table.BranchRef, 0, 0, 0)
+ reqs := []table.Requirement{table.AssertTableUUID(meta.TableUUID())}
Review Comment:
Requirements slice is just `AssertTableUUID`. A concurrent writer advancing
`main` between `loadTable` and `CommitTable` will be silently clobbered.
`previousSnapshotID` is already computed for the result struct — pass it
through as `table.AssertRefSnapshotID(table.MainBranch, previousSnapshotID)`
here:
```go
reqs := []table.Requirement{
table.AssertTableUUID(meta.TableUUID()),
table.AssertRefSnapshotID(table.MainBranch, previousSnapshotID),
}
```
PyIceberg's `_set_ref_snapshot` does this unconditionally; both Java's
`SetSnapshotOperation` and the iceberg-go transaction layer use the same CAS
pattern. Without it, this rollback is destructive under any multi-writer setup.
##########
cmd/iceberg/upgrade_rollback.go:
##########
@@ -19,23 +19,144 @@ package main
import (
"context"
- "errors"
+ "encoding/json"
+ "fmt"
"os"
+ "strconv"
"github.com/apache/iceberg-go/catalog"
+ "github.com/apache/iceberg-go/table"
+ "github.com/pterm/pterm"
)
-func runUpgrade(_ context.Context, output Output, _ catalog.Catalog, _
*UpgradeCmd) {
- output.Error(errors.New("upgrade: not yet implemented"))
- os.Exit(1)
+func runUpgrade(ctx context.Context, output Output, cat catalog.Catalog, cmd
*UpgradeCmd) {
+ tbl := loadTable(ctx, output, cat, cmd.TableID)
+ meta := tbl.Metadata()
+ currentVersion := meta.Version()
+
+ if cmd.FormatVersion <= currentVersion {
+ output.Error(fmt.Errorf("target format version %d must be
greater than current version %d",
+ cmd.FormatVersion, currentVersion))
+ os.Exit(1)
+ }
+
+ result := UpgradeResult{
+ DryRun: cmd.DryRun,
+ Table: tableIDString(tbl),
+ PreviousVersion: currentVersion,
+ TargetVersion: cmd.FormatVersion,
+ SpecURL: specURL(cmd.FormatVersion),
+ }
+
+ if cmd.DryRun {
+ output.UpgradeResult(result)
+
+ return
+ }
+
+ prompt := fmt.Sprintf("Upgrade %s from format version %d to %d?",
+ tableIDString(tbl), currentVersion, cmd.FormatVersion)
+ if err := confirmAction(prompt, cmd.Yes); err != nil {
+ output.Error(err)
+ os.Exit(1)
+ }
+
+ tx := tbl.NewTransaction()
+ if err := tx.UpgradeFormatVersion(cmd.FormatVersion); err != nil {
+ output.Error(fmt.Errorf("upgrade failed: %w", err))
+ os.Exit(1)
+ }
+
+ if _, err := tx.Commit(ctx); err != nil {
+ output.Error(fmt.Errorf("commit failed: %w", err))
+ os.Exit(1)
+ }
+
+ output.UpgradeResult(result)
+}
+
+func runRollback(ctx context.Context, output Output, cat catalog.Catalog, cmd
*RollbackCmd) {
+ tbl := loadTable(ctx, output, cat, cmd.TableID)
+ meta := tbl.Metadata()
+
+ snap := meta.SnapshotByID(cmd.SnapshotID)
+ if snap == nil {
+ output.Error(fmt.Errorf("snapshot %d not found in table %s",
cmd.SnapshotID, tableIDString(tbl)))
+ os.Exit(1)
+ }
+
+ var previousSnapshotID *int64
+ if cs := meta.CurrentSnapshot(); cs != nil {
+ id := cs.SnapshotID
+ previousSnapshotID = &id
+ }
+
+ prompt := fmt.Sprintf("Roll back %s to snapshot %d?",
tableIDString(tbl), cmd.SnapshotID)
+ if err := confirmAction(prompt, cmd.Yes); err != nil {
+ output.Error(err)
+ os.Exit(1)
+ }
+
+ update := table.NewSetSnapshotRefUpdate(table.MainBranch,
cmd.SnapshotID, table.BranchRef, 0, 0, 0)
+ reqs := []table.Requirement{table.AssertTableUUID(meta.TableUUID())}
+
+ if _, _, err := cat.CommitTable(ctx, tbl.Identifier(), reqs,
[]table.Update{update}); err != nil {
Review Comment:
Going through `cat.CommitTable` directly here (rather than
`tbl.NewTransaction()` like `runUpgrade` does) also drops the OCC retry loop
and `rewriteRefSnapshotRequirements` that `Transaction.Commit` provides. Two
options: lift this through a new `Transaction.RollbackToSnapshot(ctx, id)` that
bakes in the CAS + ancestry checks (preferred — mirrors the
`UpgradeFormatVersion` pattern), or leave a `TODO` here noting the asymmetry
for a follow-up.
##########
cmd/iceberg/upgrade_rollback_test.go:
##########
@@ -0,0 +1,166 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package main
+
+import (
+ "bytes"
+ "os"
+ "testing"
+
+ "github.com/pterm/pterm"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestSpecURL(t *testing.T) {
+ assert.Contains(t, specURL(1), "version-1")
+ assert.Contains(t, specURL(2), "version-2")
+ assert.Contains(t, specURL(3), "version-3")
+ assert.Equal(t, "https://iceberg.apache.org/spec/";, specURL(99))
+}
+
+func TestTextOutputUpgradeResultDryRun(t *testing.T) {
+ var buf bytes.Buffer
+ pterm.SetDefaultOutput(&buf)
Review Comment:
`pterm.SetDefaultOutput(&buf)` mutates package-global state without restore
(same pattern at lines 60, 107, 127). Fine while everything's sequential, but
the moment any test adds `t.Parallel()` the output sink is a data race. Add
`t.Cleanup(func() { pterm.SetDefaultOutput(os.Stdout); pterm.EnableColor() })`
per test.
##########
cmd/iceberg/upgrade_rollback.go:
##########
@@ -19,23 +19,144 @@ package main
import (
"context"
- "errors"
+ "encoding/json"
+ "fmt"
"os"
+ "strconv"
"github.com/apache/iceberg-go/catalog"
+ "github.com/apache/iceberg-go/table"
+ "github.com/pterm/pterm"
)
-func runUpgrade(_ context.Context, output Output, _ catalog.Catalog, _
*UpgradeCmd) {
- output.Error(errors.New("upgrade: not yet implemented"))
- os.Exit(1)
+func runUpgrade(ctx context.Context, output Output, cat catalog.Catalog, cmd
*UpgradeCmd) {
+ tbl := loadTable(ctx, output, cat, cmd.TableID)
+ meta := tbl.Metadata()
+ currentVersion := meta.Version()
+
+ if cmd.FormatVersion <= currentVersion {
Review Comment:
The `cmd.FormatVersion <= currentVersion` guard fires before the dry-run
branch, so `upgrade TABLE 1 --dry-run` on a v2 table exits 1 instead of
producing the promised `[DRY RUN]` no-op preview. Move the guard inside
`!cmd.DryRun`, or render a "no-op, already at version N" dry-run line. The
dry-run contract in #957 requires exit 0 with a preview.
##########
cmd/iceberg/upgrade_rollback.go:
##########
@@ -19,23 +19,144 @@ package main
import (
"context"
- "errors"
+ "encoding/json"
+ "fmt"
"os"
+ "strconv"
"github.com/apache/iceberg-go/catalog"
+ "github.com/apache/iceberg-go/table"
+ "github.com/pterm/pterm"
)
-func runUpgrade(_ context.Context, output Output, _ catalog.Catalog, _
*UpgradeCmd) {
- output.Error(errors.New("upgrade: not yet implemented"))
- os.Exit(1)
+func runUpgrade(ctx context.Context, output Output, cat catalog.Catalog, cmd
*UpgradeCmd) {
+ tbl := loadTable(ctx, output, cat, cmd.TableID)
+ meta := tbl.Metadata()
+ currentVersion := meta.Version()
+
+ if cmd.FormatVersion <= currentVersion {
+ output.Error(fmt.Errorf("target format version %d must be
greater than current version %d",
+ cmd.FormatVersion, currentVersion))
+ os.Exit(1)
+ }
+
+ result := UpgradeResult{
+ DryRun: cmd.DryRun,
+ Table: tableIDString(tbl),
+ PreviousVersion: currentVersion,
+ TargetVersion: cmd.FormatVersion,
+ SpecURL: specURL(cmd.FormatVersion),
+ }
+
+ if cmd.DryRun {
+ output.UpgradeResult(result)
+
+ return
+ }
+
+ prompt := fmt.Sprintf("Upgrade %s from format version %d to %d?",
+ tableIDString(tbl), currentVersion, cmd.FormatVersion)
+ if err := confirmAction(prompt, cmd.Yes); err != nil {
+ output.Error(err)
+ os.Exit(1)
+ }
+
+ tx := tbl.NewTransaction()
+ if err := tx.UpgradeFormatVersion(cmd.FormatVersion); err != nil {
+ output.Error(fmt.Errorf("upgrade failed: %w", err))
+ os.Exit(1)
+ }
+
+ if _, err := tx.Commit(ctx); err != nil {
+ output.Error(fmt.Errorf("commit failed: %w", err))
+ os.Exit(1)
+ }
+
+ output.UpgradeResult(result)
+}
+
+func runRollback(ctx context.Context, output Output, cat catalog.Catalog, cmd
*RollbackCmd) {
+ tbl := loadTable(ctx, output, cat, cmd.TableID)
+ meta := tbl.Metadata()
+
+ snap := meta.SnapshotByID(cmd.SnapshotID)
+ if snap == nil {
+ output.Error(fmt.Errorf("snapshot %d not found in table %s",
cmd.SnapshotID, tableIDString(tbl)))
+ os.Exit(1)
+ }
+
+ var previousSnapshotID *int64
+ if cs := meta.CurrentSnapshot(); cs != nil {
+ id := cs.SnapshotID
+ previousSnapshotID = &id
+ }
+
+ prompt := fmt.Sprintf("Roll back %s to snapshot %d?",
tableIDString(tbl), cmd.SnapshotID)
+ if err := confirmAction(prompt, cmd.Yes); err != nil {
+ output.Error(err)
+ os.Exit(1)
+ }
+
+ update := table.NewSetSnapshotRefUpdate(table.MainBranch,
cmd.SnapshotID, table.BranchRef, 0, 0, 0)
+ reqs := []table.Requirement{table.AssertTableUUID(meta.TableUUID())}
+
+ if _, _, err := cat.CommitTable(ctx, tbl.Identifier(), reqs,
[]table.Update{update}); err != nil {
+ output.Error(fmt.Errorf("rollback failed: %w", err))
+ os.Exit(1)
+ }
+
+ result := RollbackResult{
+ Table: tableIDString(tbl),
+ PreviousSnapshotID: previousSnapshotID,
+ RolledBackToSnapshotID: cmd.SnapshotID,
+ }
+
+ output.RollbackResult(result)
}
-func runRollback(_ context.Context, output Output, _ catalog.Catalog, _
*RollbackCmd) {
- output.Error(errors.New("rollback: not yet implemented"))
- os.Exit(1)
+func specURL(version int) string {
+ switch version {
+ case 1:
+ return
"https://iceberg.apache.org/spec/#version-1-analytic-data-tables";
+ case 2:
+ return
"https://iceberg.apache.org/spec/#version-2-row-level-deletes";
+ case 3:
+ return
"https://iceberg.apache.org/spec/#version-3-extended-types-and-features";
+ default:
+ return "https://iceberg.apache.org/spec/";
+ }
+}
+
+func (t textOutput) UpgradeResult(result UpgradeResult) {
Review Comment:
Receiver named `t` but unused, and `t` universally means `*testing.T` in
this `package main` where tests live alongside production code. The established
pattern in `output.go` is a blank receiver for methods that don't read the
receiver: `func (textOutput) UpgradeResult(...)`. Same applies to
`RollbackResult` below.
##########
cmd/iceberg/upgrade_rollback.go:
##########
@@ -19,23 +19,144 @@ package main
import (
"context"
- "errors"
+ "encoding/json"
+ "fmt"
"os"
+ "strconv"
"github.com/apache/iceberg-go/catalog"
+ "github.com/apache/iceberg-go/table"
+ "github.com/pterm/pterm"
)
-func runUpgrade(_ context.Context, output Output, _ catalog.Catalog, _
*UpgradeCmd) {
- output.Error(errors.New("upgrade: not yet implemented"))
- os.Exit(1)
+func runUpgrade(ctx context.Context, output Output, cat catalog.Catalog, cmd
*UpgradeCmd) {
+ tbl := loadTable(ctx, output, cat, cmd.TableID)
+ meta := tbl.Metadata()
+ currentVersion := meta.Version()
+
+ if cmd.FormatVersion <= currentVersion {
+ output.Error(fmt.Errorf("target format version %d must be
greater than current version %d",
+ cmd.FormatVersion, currentVersion))
+ os.Exit(1)
+ }
+
+ result := UpgradeResult{
+ DryRun: cmd.DryRun,
+ Table: tableIDString(tbl),
+ PreviousVersion: currentVersion,
+ TargetVersion: cmd.FormatVersion,
+ SpecURL: specURL(cmd.FormatVersion),
+ }
+
+ if cmd.DryRun {
+ output.UpgradeResult(result)
+
+ return
+ }
+
+ prompt := fmt.Sprintf("Upgrade %s from format version %d to %d?",
+ tableIDString(tbl), currentVersion, cmd.FormatVersion)
+ if err := confirmAction(prompt, cmd.Yes); err != nil {
+ output.Error(err)
+ os.Exit(1)
+ }
+
+ tx := tbl.NewTransaction()
+ if err := tx.UpgradeFormatVersion(cmd.FormatVersion); err != nil {
+ output.Error(fmt.Errorf("upgrade failed: %w", err))
+ os.Exit(1)
+ }
+
+ if _, err := tx.Commit(ctx); err != nil {
+ output.Error(fmt.Errorf("commit failed: %w", err))
+ os.Exit(1)
+ }
+
+ output.UpgradeResult(result)
+}
+
+func runRollback(ctx context.Context, output Output, cat catalog.Catalog, cmd
*RollbackCmd) {
+ tbl := loadTable(ctx, output, cat, cmd.TableID)
+ meta := tbl.Metadata()
+
+ snap := meta.SnapshotByID(cmd.SnapshotID)
+ if snap == nil {
+ output.Error(fmt.Errorf("snapshot %d not found in table %s",
cmd.SnapshotID, tableIDString(tbl)))
+ os.Exit(1)
+ }
Review Comment:
`meta.SnapshotByID(cmd.SnapshotID) != nil` proves the snapshot exists but
not that it's on the current main ancestry chain. As written, `rollback
--snapshot-id <sibling-branch-head>` succeeds and re-points `main` at an
unrelated history — downstream readers see a `snapshot-log` with a gap. Java's
`ManageSnapshots.rollbackTo` and PyIceberg's `rollback_to_snapshot` both reject
non-ancestors before committing.
`table.IsAncestorOf` is already exported from `table/snapshot_ancestry.go`.
Suggested:
```go
if cs := meta.CurrentSnapshot(); cs != nil {
if !table.IsAncestorOf(cs.SnapshotID, cmd.SnapshotID, meta.SnapshotByID)
{
output.Error(fmt.Errorf("snapshot %d is not an ancestor of current
snapshot %d", cmd.SnapshotID, cs.SnapshotID))
os.Exit(1)
}
}
```
##########
cmd/iceberg/upgrade_rollback_test.go:
##########
@@ -0,0 +1,166 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package main
+
+import (
+ "bytes"
+ "os"
+ "testing"
+
+ "github.com/pterm/pterm"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestSpecURL(t *testing.T) {
+ assert.Contains(t, specURL(1), "version-1")
+ assert.Contains(t, specURL(2), "version-2")
+ assert.Contains(t, specURL(3), "version-3")
+ assert.Equal(t, "https://iceberg.apache.org/spec/";, specURL(99))
+}
+
+func TestTextOutputUpgradeResultDryRun(t *testing.T) {
+ var buf bytes.Buffer
+ pterm.SetDefaultOutput(&buf)
+ pterm.DisableColor()
+
+ result := UpgradeResult{
+ DryRun: true,
+ Table: "db.events",
+ PreviousVersion: 1,
+ TargetVersion: 2,
+ SpecURL: specURL(2),
+ }
+
+ buf.Reset()
+ textOutput{}.UpgradeResult(result)
+
+ output := buf.String()
+ assert.Contains(t, output, "[DRY RUN]")
+ assert.Contains(t, output, "format version 1 to 2")
+ assert.Contains(t, output, "version-2")
+}
+
+func TestTextOutputUpgradeResultCommitted(t *testing.T) {
+ var buf bytes.Buffer
+ pterm.SetDefaultOutput(&buf)
+ pterm.DisableColor()
+
+ result := UpgradeResult{
+ DryRun: false,
+ Table: "db.events",
+ PreviousVersion: 1,
+ TargetVersion: 2,
+ SpecURL: specURL(2),
+ }
+
+ buf.Reset()
+ textOutput{}.UpgradeResult(result)
+
+ output := buf.String()
+ assert.Contains(t, output, "Upgraded db.events")
+ assert.Contains(t, output, "format version 1 to 2")
+}
+
+func TestJSONOutputUpgradeResult(t *testing.T) {
+ oldStdout := os.Stdout
+ r, w, _ := os.Pipe()
Review Comment:
`r, w, _ := os.Pipe()` swallows the error; on pipe-creation failure, `w` is
nil and the subsequent `os.Stdout = w` corrupts the process for any later test.
Same issue in `TestJSONOutputRollbackResult` at line 145. Suggested:
```go
r, w, err := os.Pipe()
require.NoError(t, err)
oldStdout := os.Stdout
os.Stdout = w
t.Cleanup(func() { w.Close(); os.Stdout = oldStdout })
```
##########
cmd/iceberg/upgrade_rollback_test.go:
##########
@@ -0,0 +1,166 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package main
+
+import (
+ "bytes"
+ "os"
+ "testing"
+
+ "github.com/pterm/pterm"
+ "github.com/stretchr/testify/assert"
+)
+
+func TestSpecURL(t *testing.T) {
+ assert.Contains(t, specURL(1), "version-1")
+ assert.Contains(t, specURL(2), "version-2")
+ assert.Contains(t, specURL(3), "version-3")
+ assert.Equal(t, "https://iceberg.apache.org/spec/";, specURL(99))
+}
+
+func TestTextOutputUpgradeResultDryRun(t *testing.T) {
+ var buf bytes.Buffer
+ pterm.SetDefaultOutput(&buf)
+ pterm.DisableColor()
+
+ result := UpgradeResult{
+ DryRun: true,
+ Table: "db.events",
+ PreviousVersion: 1,
+ TargetVersion: 2,
+ SpecURL: specURL(2),
+ }
+
+ buf.Reset()
+ textOutput{}.UpgradeResult(result)
+
+ output := buf.String()
+ assert.Contains(t, output, "[DRY RUN]")
+ assert.Contains(t, output, "format version 1 to 2")
+ assert.Contains(t, output, "version-2")
+}
+
+func TestTextOutputUpgradeResultCommitted(t *testing.T) {
+ var buf bytes.Buffer
+ pterm.SetDefaultOutput(&buf)
+ pterm.DisableColor()
+
+ result := UpgradeResult{
+ DryRun: false,
+ Table: "db.events",
+ PreviousVersion: 1,
+ TargetVersion: 2,
+ SpecURL: specURL(2),
+ }
+
+ buf.Reset()
+ textOutput{}.UpgradeResult(result)
+
+ output := buf.String()
+ assert.Contains(t, output, "Upgraded db.events")
+ assert.Contains(t, output, "format version 1 to 2")
+}
+
+func TestJSONOutputUpgradeResult(t *testing.T) {
Review Comment:
Coverage here is formatter-only — neither `runRollback` nor `runUpgrade` is
exercised end-to-end. The two contract lines from #957 ("verify no catalog
commit in dry-run", "non-TTY without `--yes` errors") aren't tested. A fake
`catalog.Catalog` whose `CommitTable` panics would catch the dry-run violation
without needing Docker.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
