Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

test: Replace t.fatalf with testify require #16038

Merged
merged 9 commits into from
Jun 14, 2024
Merged

test: Replace t.fatalf with testify require #16038

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e0eee4f
fix
Ari1009 Jun 2, 2024
6b67e76
update
Ari1009 Jun 3, 2024
91d9c25
Update_2
Ari1009 Jun 3, 2024
4239566
Updates_3
Ari1009 Jun 3, 2024
3c10f00
Update_4
Ari1009 Jun 4, 2024
6b4b485
Remove_UnecessaryPrint
Ari1009 Jun 4, 2024
e03a735
cleanup
Ari1009 Jun 4, 2024
49876f3
Cleanup_2
Ari1009 Jun 5, 2024
ccdc5f1
Merge branch 'vitessio:main' into fix
Ari1009 Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions go/tools/asthelpergen/asthelpergen_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ 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,
distributed under the License is distributed on an "AS IS" BASIS,
Ari1009 marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand Down Expand Up @@ -42,8 +42,6 @@ func TestFullGeneration(t *testing.T) {
require.Contains(t, contents, "http://www.apache.org/licenses/LICENSE-2.0")
applyIdx := strings.Index(contents, "func (a *application) apply(parent, node AST, replacer replacerFunc)")
cloneIdx := strings.Index(contents, "CloneAST(in AST) AST")
if applyIdx == 0 && cloneIdx == 0 {
t.Fatalf("file doesn't contain expected contents")
}
require.False(t, applyIdx == 0 && cloneIdx == 0, "file doesn't contain expected contents")
}
}
8 changes: 2 additions & 6 deletions go/trace/trace_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,9 @@ func TestRegisterService(t *testing.T) {
serviceName := "vtservice"
closer := StartTracing(serviceName)
tracer, ok := closer.(*fakeTracer)
if !ok {
t.Fatalf("did not get the expected tracer, got %+v (%T)", tracer, tracer)
}
require.True(t, ok, fmt.Sprintf("did not get the expected tracer, got %+v (%T)", tracer, tracer))
Ari1009 marked this conversation as resolved.
Show resolved Hide resolved
require.Equal(t, serviceName, tracer.name)

if tracer.name != serviceName {
t.Fatalf("expected the name to be `%v` but it was `%v`", serviceName, tracer.name)
}
}

func TestNewFromString(t *testing.T) {
Expand Down
33 changes: 6 additions & 27 deletions go/vt/binlog/binlog_streamer_rbr_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ package binlog

import (
"context"
"reflect"
"testing"

"github.com/stretchr/testify/assert"

"vitess.io/vitess/go/mysql"
"vitess.io/vitess/go/mysql/binlog"
"vitess.io/vitess/go/mysql/collations"
Expand Down Expand Up @@ -270,19 +271,8 @@ func TestStreamerParseRBREvents(t *testing.T) {

go sendTestEvents(events, input)
_, err := bls.parseEvents(context.Background(), events, errs)
if err != ErrServerEOF {
t.Errorf("unexpected error: %v", err)
}

if !reflect.DeepEqual(got, want) {
t.Errorf("binlogConnStreamer.parseEvents(): got:\n%+v\nwant:\n%+v", got, want)
for i, fbt := range got {
t.Errorf("Got (%v)=%v", i, fbt.statements)
}
for i, fbt := range want {
t.Errorf("Want(%v)=%v", i, fbt.statements)
}
}
assert.EqualError(t, err, ErrServerEOF.Error(), "unexpected error")
Ari1009 marked this conversation as resolved.
Show resolved Hide resolved
assert.Equal(t, want, got, "binlogConnStreamer.parseEvents()")
}

func TestStreamerParseRBRNameEscapes(t *testing.T) {
Expand Down Expand Up @@ -519,17 +509,6 @@ func TestStreamerParseRBRNameEscapes(t *testing.T) {

go sendTestEvents(events, input)
_, err := bls.parseEvents(context.Background(), events, errs)
if err != ErrServerEOF {
t.Errorf("unexpected error: %v", err)
}

if !reflect.DeepEqual(got, want) {
t.Errorf("binlogConnStreamer.parseEvents(): got:\n%+v\nwant:\n%+v", got, want)
for i, fbt := range got {
t.Errorf("Got (%v)=%v", i, fbt.statements)
}
for i, fbt := range want {
t.Errorf("Want(%v)=%v", i, fbt.statements)
}
}
assert.EqualError(t, err, ErrServerEOF.Error(), "unexpected error")
assert.Equal(t, want, got, "binlogConnStreamer.parseEvents()")
}
18 changes: 6 additions & 12 deletions go/vt/binlog/tables_filter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"

binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata"
querypb "vitess.io/vitess/go/vt/proto/query"
)
Expand Down Expand Up @@ -60,9 +62,7 @@ func TestTablesFilterPass(t *testing.T) {
})
_ = f(eventToken, statements)
want := `statement: <6, "set1"> statement: <7, "dml1 /* _stream included1 (id ) (500 ); */"> statement: <7, "dml2 /* _stream included2 (id ) (500 ); */"> position: "MariaDB/0-41983-1" `
if want != got {
t.Errorf("want\n%s, got\n%s", want, got)
}
assert.Equal(t, want, got, "binlogConnStreamer.tablesFilterFunc()")
}

func TestTablesFilterSkip(t *testing.T) {
Expand Down Expand Up @@ -90,9 +90,7 @@ func TestTablesFilterSkip(t *testing.T) {
})
_ = f(eventToken, statements)
want := `position: "MariaDB/0-41983-1" `
if want != got {
t.Errorf("want %s, got %s", want, got)
}
assert.Equal(t, want, got, "binlogConnStreamer.tablesFilterFunc()")
}

func TestTablesFilterDDL(t *testing.T) {
Expand Down Expand Up @@ -120,9 +118,7 @@ func TestTablesFilterDDL(t *testing.T) {
})
_ = f(eventToken, statements)
want := `position: "MariaDB/0-41983-1" `
if want != got {
t.Errorf("want %s, got %s", want, got)
}
assert.Equal(t, want, got, "binlogConnStreamer.tablesFilterFunc()")
}

func TestTablesFilterMalformed(t *testing.T) {
Expand Down Expand Up @@ -156,9 +152,7 @@ func TestTablesFilterMalformed(t *testing.T) {
})
_ = f(eventToken, statements)
want := `position: "MariaDB/0-41983-1" `
if want != got {
t.Errorf("want %s, got %s", want, got)
}
assert.Equal(t, want, got, "binlogConnStreamer.tablesFilterFunc()")
}

func bltToString(tx *binlogdatapb.BinlogTransaction) string {
Expand Down
115 changes: 47 additions & 68 deletions go/vt/wrangler/tablet_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ package wrangler

import (
"context"
"strings"
"testing"

"github.com/stretchr/testify/require"

"vitess.io/vitess/go/vt/logutil"
topodatapb "vitess.io/vitess/go/vt/proto/topodata"
"vitess.io/vitess/go/vt/topo"
Expand Down Expand Up @@ -48,20 +49,14 @@ func TestInitTabletShardConversion(t *testing.T) {
Shard: "80-C0",
}

if err := wr.TopoServer().InitTablet(context.Background(), tablet, false /*allowPrimaryOverride*/, true /*createShardAndKeyspace*/, false /*allowUpdate*/); err != nil {
t.Fatalf("InitTablet failed: %v", err)
}
err := wr.TopoServer().InitTablet(context.Background(), tablet, false /*allowPrimaryOverride*/, true /*createShardAndKeyspace*/, false /*allowUpdate*/)
require.NoError(t, err, "InitTablet failed")
Ari1009 marked this conversation as resolved.
Show resolved Hide resolved

ti, err := ts.GetTablet(context.Background(), tablet.Alias)
if err != nil {
t.Fatalf("GetTablet failed: %v", err)
}
if ti.Shard != "80-c0" {
t.Errorf("Got wrong tablet.Shard, got %v expected 80-c0", ti.Shard)
}
if string(ti.KeyRange.Start) != "\x80" || string(ti.KeyRange.End) != "\xc0" {
t.Errorf("Got wrong tablet.KeyRange, got %v expected 80-c0", ti.KeyRange)
}
require.NoError(t, err, "GetTablet failed")
require.Equal(t, "80-c0", ti.Shard, "Got wrong tablet.Shard")
require.Equal(t, "\x80", string(ti.KeyRange.Start), "Got wrong tablet.KeyRange start")
require.Equal(t, "\xc0", string(ti.KeyRange.End), "Got wrong tablet.KeyRange end")
Ari1009 marked this conversation as resolved.
Show resolved Hide resolved
}

// TestDeleteTabletBasic tests delete of non-primary tablet
Expand All @@ -82,17 +77,14 @@ func TestDeleteTabletBasic(t *testing.T) {
Keyspace: "test",
}

if err := wr.TopoServer().InitTablet(context.Background(), tablet, false /*allowPrimaryOverride*/, true /*createShardAndKeyspace*/, false /*allowUpdate*/); err != nil {
t.Fatalf("InitTablet failed: %v", err)
}
err := wr.TopoServer().InitTablet(context.Background(), tablet, false /*allowPrimaryOverride*/, true /*createShardAndKeyspace*/, false /*allowUpdate*/)
require.NoError(t, err, "InitTablet failed")

if _, err := ts.GetTablet(context.Background(), tablet.Alias); err != nil {
t.Fatalf("GetTablet failed: %v", err)
}
_, err = ts.GetTablet(context.Background(), tablet.Alias)
require.NoError(t, err, "GetTablet failed")

if err := wr.DeleteTablet(context.Background(), tablet.Alias, false); err != nil {
t.Fatalf("DeleteTablet failed: %v", err)
}
err = wr.DeleteTablet(context.Background(), tablet.Alias, false)
require.NoError(t, err, "DeleteTablet failed")
Ari1009 marked this conversation as resolved.
Show resolved Hide resolved
}

// TestDeleteTabletTruePrimary tests that you can delete a true primary tablet
Expand All @@ -115,31 +107,26 @@ func TestDeleteTabletTruePrimary(t *testing.T) {
Type: topodatapb.TabletType_PRIMARY,
}

if err := wr.TopoServer().InitTablet(context.Background(), tablet, false /*allowPrimaryOverride*/, true /*createShardAndKeyspace*/, false /*allowUpdate*/); err != nil {
t.Fatalf("InitTablet failed: %v", err)
}
if _, err := ts.GetTablet(context.Background(), tablet.Alias); err != nil {
t.Fatalf("GetTablet failed: %v", err)
}
err := wr.TopoServer().InitTablet(context.Background(), tablet, false /*allowPrimaryOverride*/, true /*createShardAndKeyspace*/, false /*allowUpdate*/)
require.NoError(t, err, "InitTablet failed")

_, err = ts.GetTablet(context.Background(), tablet.Alias)
require.NoError(t, err, "GetTablet failed")

// set PrimaryAlias and PrimaryTermStartTime on shard to match chosen primary tablet
if _, err := ts.UpdateShardFields(context.Background(), "test", "0", func(si *topo.ShardInfo) error {
_, err = ts.UpdateShardFields(context.Background(), "test", "0", func(si *topo.ShardInfo) error {
si.PrimaryAlias = tablet.Alias
si.PrimaryTermStartTime = tablet.PrimaryTermStartTime
return nil
}); err != nil {
t.Fatalf("UpdateShardFields failed: %v", err)
}
})
require.NoError(t, err, "UpdateShardFields failed")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the last string here either.


err := wr.DeleteTablet(context.Background(), tablet.Alias, false)
err = wr.DeleteTablet(context.Background(), tablet.Alias, false)
wantError := "as it is a primary, use allow_primary flag"
if err == nil || !strings.Contains(err.Error(), wantError) {
t.Fatalf("DeleteTablet on primary: want error = %v, got error = %v", wantError, err)
}
require.ErrorContains(t, err, wantError, "DeleteTablet on primary: want specific error message")

if err := wr.DeleteTablet(context.Background(), tablet.Alias, true); err != nil {
t.Fatalf("DeleteTablet failed: %v", err)
}
err = wr.DeleteTablet(context.Background(), tablet.Alias, true)
require.NoError(t, err, "DeleteTablet failed")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ari1009 I think you can remove this one.

}

// TestDeleteTabletFalsePrimary tests that you can delete a false primary tablet
Expand All @@ -162,9 +149,8 @@ func TestDeleteTabletFalsePrimary(t *testing.T) {
Type: topodatapb.TabletType_PRIMARY,
}

if err := wr.TopoServer().InitTablet(context.Background(), tablet1, false /*allowPrimaryOverride*/, true /*createShardAndKeyspace*/, false /*allowUpdate*/); err != nil {
t.Fatalf("InitTablet failed: %v", err)
}
err := wr.TopoServer().InitTablet(context.Background(), tablet1, false /*allowPrimaryOverride*/, true /*createShardAndKeyspace*/, false /*allowUpdate*/)
require.NoError(t, err, "InitTablet failed")

tablet2 := &topodatapb.Tablet{
Alias: &topodatapb.TabletAlias{
Expand All @@ -175,23 +161,20 @@ func TestDeleteTabletFalsePrimary(t *testing.T) {
Shard: "0",
Type: topodatapb.TabletType_PRIMARY,
}
if err := wr.TopoServer().InitTablet(context.Background(), tablet2, true /*allowPrimaryOverride*/, false /*createShardAndKeyspace*/, false /*allowUpdate*/); err != nil {
t.Fatalf("InitTablet failed: %v", err)
}
err = wr.TopoServer().InitTablet(context.Background(), tablet2, true /*allowPrimaryOverride*/, false /*createShardAndKeyspace*/, false /*allowUpdate*/)
require.NoError(t, err, "InitTablet failed")

// set PrimaryAlias and PrimaryTermStartTime on shard to match chosen primary tablet
if _, err := ts.UpdateShardFields(context.Background(), "test", "0", func(si *topo.ShardInfo) error {
_, err = ts.UpdateShardFields(context.Background(), "test", "0", func(si *topo.ShardInfo) error {
si.PrimaryAlias = tablet2.Alias
si.PrimaryTermStartTime = tablet2.PrimaryTermStartTime
return nil
}); err != nil {
t.Fatalf("UpdateShardFields failed: %v", err)
}
})
require.NoError(t, err, "UpdateShardFields failed")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here as well.


// Should be able to delete old (false) primary with allowPrimary = false
if err := wr.DeleteTablet(context.Background(), tablet1.Alias, false); err != nil {
t.Fatalf("DeleteTablet failed: %v", err)
}
err = wr.DeleteTablet(context.Background(), tablet1.Alias, false)
require.NoError(t, err, "DeleteTablet failed")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too.

}

// TestDeleteTabletShardNonExisting tests that you can delete a true primary
Expand All @@ -214,29 +197,25 @@ func TestDeleteTabletShardNonExisting(t *testing.T) {
Type: topodatapb.TabletType_PRIMARY,
}

if err := wr.TopoServer().InitTablet(context.Background(), tablet, false /*allowPrimaryOverride*/, true /*createShardAndKeyspace*/, false /*allowUpdate*/); err != nil {
t.Fatalf("InitTablet failed: %v", err)
}
if _, err := ts.GetTablet(context.Background(), tablet.Alias); err != nil {
t.Fatalf("GetTablet failed: %v", err)
}
err := wr.TopoServer().InitTablet(context.Background(), tablet, false /*allowPrimaryOverride*/, true /*createShardAndKeyspace*/, false /*allowUpdate*/)
require.NoError(t, err, "InitTablet failed")

_, err = ts.GetTablet(context.Background(), tablet.Alias)
require.NoError(t, err, "GetTablet failed")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the additional string here.


// set PrimaryAlias and PrimaryTermStartTime on shard to match chosen primary tablet
if _, err := ts.UpdateShardFields(context.Background(), "test", "0", func(si *topo.ShardInfo) error {
_, err = ts.UpdateShardFields(context.Background(), "test", "0", func(si *topo.ShardInfo) error {
si.PrimaryAlias = tablet.Alias
si.PrimaryTermStartTime = tablet.PrimaryTermStartTime
return nil
}); err != nil {
t.Fatalf("UpdateShardFields failed: %v", err)
}
})
require.NoError(t, err, "UpdateShardFields failed")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need here either.


// trigger a shard deletion
if err := ts.DeleteShard(context.Background(), "test", "0"); err != nil {
t.Fatalf("DeleteShard failed: %v", err)
}
err = ts.DeleteShard(context.Background(), "test", "0")
require.NoError(t, err, "DeleteShard failed")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need here either.


// DeleteTablet should not fail if a shard no longer exist
if err := wr.DeleteTablet(context.Background(), tablet.Alias, true); err != nil {
t.Fatalf("DeleteTablet failed: %v", err)
}
err = wr.DeleteTablet(context.Background(), tablet.Alias, true)
require.NoError(t, err, "DeleteTablet failed")
}
11 changes: 4 additions & 7 deletions go/vt/wrangler/vexec_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"vitess.io/vitess/go/sqltypes"
Expand Down Expand Up @@ -151,15 +152,11 @@ func TestVExec(t *testing.T) {
if testCase.errorString == "" {
require.NoError(t, err)
for _, result := range results {
if !testCase.result.Equal(result) {
t.Errorf("mismatched result:\nwant: %v\ngot: %v", testCase.result, result)
}
assert.True(t, testCase.result.Equal(result), "mismatched result")
}
} else {
require.Error(t, err)
if !strings.Contains(err.Error(), testCase.errorString) {
t.Fatalf("Wrong error, want %s, got %s", testCase.errorString, err.Error())
}
require.ErrorContains(t, err, testCase.errorString, "Wrong error, want %s, got %s", testCase.errorString, err.Error())

}
})
}
Expand Down
Loading
Loading