Merge pull request #357 from inada-s/fix/test-low-priority-improvements

Improve test cleanup, error coverage, and assertion messages

Author: inada-s

CLAUDE.md

@@ -130,7 +130,7 @@ See `.golangci.yml`.
 - **Branch naming**: Use prefixed format: `feature/xxx`, `fix/xxx`, `refactor/xxx`.
 - **PR target**: Always `master`.
 - **PR title**: English, under 70 characters.
-- **PR description**: English, include summary and test plan.
+- **PR description**: English, include summary and test plan. Do not include auto-generated credit lines (e.g. "Generated with Claude Code").
 - When asked to commit or create a PR, create a branch, commit, push, and open the PR via `gh pr create`.
 
 ## Cross-Repository: flycast

gdxsv/db_test.go

@@ -207,9 +207,7 @@ func Test203CalculateUserBattleCount(t *testing.T) {
 }
 
 func Test300Ranking(t *testing.T) {
-	// ugly...
-	_, err := getDB().(SQLiteDB).Exec("DELETE FROM user")
-	must(t, err)
+	cleanTables(t, "user")
 
 	var users []*DBUser
 	for i := 0; i < 3; i++ {
@@ -338,10 +336,7 @@ func Test300Ranking(t *testing.T) {
 }
 
 func Test400Replay(t *testing.T) {
-	_, err := getDB().(SQLiteDB).Exec("DELETE FROM user")
-	must(t, err)
-	_, err = getDB().(SQLiteDB).Exec("DELETE FROM battle_record")
-	must(t, err)
+	cleanTables(t, "user", "battle_record")
 
 	var users []*DBUser
 	for i := 0; i < 4; i++ {

gdxsv/lbs_battle_test.go

@@ -18,6 +18,10 @@ func Test_toIPPort(t *testing.T) {
 	}{
 		{"tcp4 addr", args{"192.168.1.10:1234"}, net.IPv4(192, 168, 1, 10), 1234, false},
 		{"localhost", args{"localhost:1234"}, net.IPv4(127, 0, 0, 1), 1234, false},
+		{"missing port", args{"192.168.1.10"}, nil, 0, true},
+		{"bad port", args{"192.168.1.10:badport"}, nil, 0, true},
+		{"empty string", args{""}, nil, 0, true},
+		{"port out of range", args{"192.168.1.10:99999"}, nil, 0, true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

gdxsv/main_test.go

@@ -22,6 +22,15 @@ func assertEq(tb testing.TB, expected, actual interface{}) {
 	}
 }
 
+func cleanTables(tb testing.TB, tables ...string) {
+	tb.Helper()
+	db := getDB().(SQLiteDB)
+	for _, table := range tables {
+		_, err := db.Exec("DELETE FROM " + table)
+		must(tb, err)
+	}
+}
+
 func waitFor(tb testing.TB, timeout time.Duration, fn func() bool) {
 	tb.Helper()
 	deadline := time.Now().Add(timeout)

gdxsv/patch_test.go

@@ -50,6 +50,41 @@ func Test_convertGamePatch(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:    "invalid size",
+			args:    args{patch: &MPatch{Disk: GameDiskDC2, Name: "bad-size", Codes: "7,0,0,0"}},
+			wantErr: true,
+		},
+		{
+			name:    "non-numeric size",
+			args:    args{patch: &MPatch{Disk: GameDiskDC2, Name: "bad-size", Codes: "abc,0,0,0"}},
+			wantErr: true,
+		},
+		{
+			name:    "invalid address",
+			args:    args{patch: &MPatch{Disk: GameDiskDC2, Name: "bad-addr", Codes: "8,zzzz,0,0"}},
+			wantErr: true,
+		},
+		{
+			name:    "invalid original value",
+			args:    args{patch: &MPatch{Disk: GameDiskDC2, Name: "bad-orig", Codes: "8,0,not_a_number,0"}},
+			wantErr: true,
+		},
+		{
+			name:    "invalid changed value",
+			args:    args{patch: &MPatch{Disk: GameDiskDC2, Name: "bad-changed", Codes: "8,0,0,not_a_number"}},
+			wantErr: true,
+		},
+		{
+			name: "empty codes",
+			args: args{patch: &MPatch{Disk: GameDiskDC2, Name: "empty", Codes: ""}},
+			want: &proto.GamePatch{GameDisk: GameDiskDC2, Name: "empty"},
+		},
+		{
+			name: "comment only",
+			args: args{patch: &MPatch{Disk: GameDiskDC2, Name: "comment-only", Codes: "# just a comment\n# another comment"}},
+			want: &proto.GamePatch{GameDisk: GameDiskDC2, Name: "comment-only"},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

gdxsv/shareddata_test.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"gdxsv/gdxsv/proto"
-	"reflect"
 	"testing"
 	"time"
 )
@@ -64,12 +63,8 @@ func TestSharedData_Sync(t *testing.T) {
 		McsGames: sd1.GetMcsGames(),
 	})
 
-	if !reflect.DeepEqual(sd1.GetMcsUsers(), sd2.GetMcsUsers()) {
-		t.Error("GetMcsUsers is different")
-	}
-	if !reflect.DeepEqual(sd1.GetMcsGames(), sd2.GetMcsGames()) {
-		t.Error("GetMcsGames is different")
-	}
+	assertEq(t, sd1.GetMcsUsers(), sd2.GetMcsUsers())
+	assertEq(t, sd1.GetMcsGames(), sd2.GetMcsGames())
 
 	sd2.UpdateMcsGameState(battleCode, McsGameStateOpened)
 	sd2.UpdateMcsUserState(sessionID, McsUserStateJoined)
@@ -82,13 +77,8 @@ func TestSharedData_Sync(t *testing.T) {
 		UpdatedAt:  time.Unix(1, 0),
 	})
 
-	if !reflect.DeepEqual(sd1.GetMcsUsers(), sd2.GetMcsUsers()) {
-		t.Error("GetMcsUsers is different")
-	}
-
-	if !reflect.DeepEqual(sd1.GetMcsGames(), sd2.GetMcsGames()) {
-		t.Error("GetMcsGames is different")
-	}
+	assertEq(t, sd1.GetMcsUsers(), sd2.GetMcsUsers())
+	assertEq(t, sd1.GetMcsGames(), sd2.GetMcsGames())
 
 	sd2.SetMcsUserCloseReason(sessionID, "timeout")
 	sd2.UpdateMcsUserState(sessionID, McsUserStateLeft)