Skip to content

Conversation

@ikura-hamu
Copy link
Member

@ikura-hamu ikura-hamu commented Jun 12, 2025

User description

fix #1089

  • ✅ handlerのGetSeatsのテスト
  • ✅ PostSeatのテスト
  • ✅ PatchSeatStatusのテスト

PR Type

Tests


Description

  • GetSeatsハンドラのユニットテスト追加

  • PostSeatハンドラのユニットテスト追加

  • PatchSeatStatusハンドラのユニットテスト追加

  • seatサービスmock生成設定追加


Changes walkthrough 📝

Relevant files
Tests
seat_test.go
seatハンドラ用テストを網羅的に追加                                                                           

src/handler/v2/seat_test.go

  • GetSeats正常系・異常系テスト追加
  • PostSeat正常系・異常系テスト追加
  • PatchSeatStatus多様なケース検証テスト追加
  • gomockによるモック生成とエラーチェック
  • +360/-0 
    Configuration changes
    seat.go
    mockgen用ディレクティブを追加                                                                             

    src/service/seat.go

    • //go:generateによるmockgenディレクティブ追加
    • モック生成先ファイルとパッケージ指定設定
    +2/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @ikura-hamu ikura-hamu requested a review from Copilot June 12, 2025 12:44
    @github-actions
    Copy link

    Migrate lint

    Lint output
    
    

    Copy link
    Contributor

    Copilot AI left a comment

    Choose a reason for hiding this comment

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

    Pull Request Overview

    This pull request adds and refines tests for seat-related handler functions (GetSeats, PostSeat, and PatchSeatStatus) while introducing a go:generate directive for creating Seat mocks.

    • Added a go:generate comment in the service package to automatically generate Seat mocks.
    • Introduced comprehensive tests covering normal and error scenarios for GetSeats, PostSeat, and PatchSeatStatus.

    Reviewed Changes

    Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

    File Description
    src/service/seat.go Added a go:generate directive to facilitate mock generation for the Seat interface.
    src/handler/v2/seat_test.go Added detailed tests for seat handlers covering various success and error cases.
    Comments suppressed due to low confidence (1)

    src/service/seat.go:3

    • Consider adding a brief note near the go:generate directive to remind developers to update the mock generation command if the Seat interface changes, ensuring the mocks stay in sync with interface updates.
    //go:generate go tool mockgen -typed -source=seat.go -destination=mock/seat.go -package=mock Seat
    

    case openapi.InUse:
    status = values.SeatStatusInUse
    default:
    t.Fatalf("invalid seat status: %v", testCase.req.Status)
    Copy link

    Copilot AI Jun 12, 2025

    Choose a reason for hiding this comment

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

    [nitpick] Consider replacing t.Fatalf with an assertion method like assert.Fail to maintain consistency with other test failure reporting patterns in the code.

    Suggested change
    t.Fatalf("invalid seat status: %v", testCase.req.Status)
    assert.Fail(t, fmt.Sprintf("invalid seat status: %v", testCase.req.Status))

    Copilot uses AI. Check for mistakes.
    @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    1089 - Fully compliant

    Compliant requirements:

    • handlerのGetSeatsのユニットテストを追加
    • handlerのPostSeatのユニットテストを追加
    • handlerのPatchSeatStatusのユニットテストを追加
    • serviceのモック生成設定を追加
    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    欠落テスト

    PostSeatPatchSeatStatusのリクエストボディが不正な場合(無効なJSONなど)へのエラーハンドリングがテストされていません。

    func TestPostSeat(t *testing.T) {
    	t.Parallel()
    
    	testCases := map[string]struct {
    		req                openapi.PostSeatRequest
    		seats              []*domain.Seat
    		UpdateSeatNumError error
    		resSeat            []*openapi.Seat
    		isError            bool
    		resStatus          int
    	}{
    		"正しく変更できる": {
    			req: openapi.PostSeatRequest{
    				Num: 3,
    			},
    			seats: []*domain.Seat{
    				domain.NewSeat(1, values.SeatStatusEmpty),
    				domain.NewSeat(2, values.SeatStatusInUse),
    				domain.NewSeat(3, values.SeatStatusEmpty),
    			},
    			resSeat: []*openapi.Seat{
    				{
    					Id:     openapi.SeatID(1),
    					Status: openapi.Empty,
    				},
    				{
    					Id:     openapi.SeatID(2),
    					Status: openapi.InUse,
    				},
    				{
    					Id:     openapi.SeatID(3),
    					Status: openapi.Empty,
    				},
    			},
    			resStatus: http.StatusOK,
    		},
    		"UpdateSeatNumがエラーなので500": {
    			req: openapi.PostSeatRequest{
    				Num: 3,
    			},
    			UpdateSeatNumError: assert.AnError,
    			isError:            true,
    			resStatus:          http.StatusInternalServerError,
    		},
    		"UpdateSeatNumで0を指定した場合": {
    			req: openapi.PostSeatRequest{
    				Num: 0,
    			},
    			seats:     []*domain.Seat{},
    			resSeat:   []*openapi.Seat{},
    			resStatus: http.StatusOK,
    		},
    	}
    
    	for name, testCase := range testCases {
    		t.Run(name, func(t *testing.T) {
    			t.Parallel()
    
    			ctrl := gomock.NewController(t)
    			seatMock := mock.NewMockSeat(ctrl)
    			seatHandler := NewSeat(seatMock)
    
    			reqBody, err := json.Marshal(testCase.req)
    			assert.NoError(t, err)
    
    			req := httptest.NewRequest(http.MethodPost, "/seats", bytes.NewBuffer(reqBody))
    			req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
    
    			rec := httptest.NewRecorder()
    			c := echo.New().NewContext(req, rec)
    
    			c.SetPath("/seats")
    
    			seatMock.EXPECT().UpdateSeatNum(gomock.Any(), uint(testCase.req.Num)).
    				Return(testCase.seats, testCase.UpdateSeatNumError)
    			err = seatHandler.PostSeat(c)
    			if testCase.isError {
    				assert.Error(t, err)
    				var httpErr *echo.HTTPError
    				if errors.As(err, &httpErr) {
    					assert.Equal(t, testCase.resStatus, httpErr.Code)
    				}
    			}
    			if err != nil {
    				return
    			}
    			assert.Equal(t, testCase.resStatus, rec.Code)
    			var res []openapi.Seat
    			err = json.NewDecoder(rec.Body).Decode(&res)
    			assert.NoError(t, err)
    			assert.Equal(t, len(testCase.resSeat), len(res))
    			for i, seat := range res {
    				assert.Equal(t, testCase.resSeat[i].Id, seat.Id)
    				assert.Equal(t, testCase.resSeat[i].Status, seat.Status)
    			}
    		})
    	}
    }
    
    func TestPatchSeatStatus(t *testing.T) {
    	t.Parallel()
    
    	testCases := map[string]struct {
    		req                     openapi.PatchSeatStatusRequest
    		seatID                  values.SeatID
    		executeUpdateSeatStatus bool
    		seat                    *domain.Seat
    		UpdateSeatStatusError   error
    		resSeat                 *openapi.Seat
    		isError                 bool
    		resStatus               int
    	}{
    		"正しく変更できる": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.InUse,
    			},
    			seatID:                  values.SeatID(1),
    			executeUpdateSeatStatus: true,
    			seat:                    domain.NewSeat(1, values.SeatStatusInUse),
    			resSeat: &openapi.Seat{
    				Id:     openapi.SeatID(1),
    				Status: openapi.InUse,
    			},
    			resStatus: http.StatusOK,
    		},
    		"無効な座席ステータスが指定されたら400": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.SeatStatus("invalid"),
    			},
    			seatID:    values.SeatID(1),
    			isError:   true,
    			resStatus: http.StatusBadRequest,
    		},
    		"UpdateSeatStatusがエラーなので500": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.InUse,
    			},
    			seatID:                  values.SeatID(1),
    			executeUpdateSeatStatus: true,
    			UpdateSeatStatusError:   assert.AnError,
    			isError:                 true,
    			resStatus:               http.StatusInternalServerError,
    		},
    		"UpdateSeatStatusで存在しない座席IDが指定されたら404": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.InUse,
    			},
    			seatID:                  values.SeatID(999),
    			executeUpdateSeatStatus: true,
    			UpdateSeatStatusError:   service.ErrNoSeat,
    			isError:                 true,
    			resStatus:               http.StatusNotFound,
    		},
    		"UpdateSeatStatusで無効な座席ステータスが指定されたら404": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.InUse,
    			},
    			seatID:                  values.SeatID(1),
    			executeUpdateSeatStatus: true,
    			UpdateSeatStatusError:   service.ErrInvalidSeatStatus,
    			isError:                 true,
    			resStatus:               http.StatusNotFound,
    		},
    		"UpdateSeatStatusで座席が空席に変更された場合": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.Empty,
    			},
    			seatID:                  values.SeatID(1),
    			executeUpdateSeatStatus: true,
    			seat:                    domain.NewSeat(1, values.SeatStatusEmpty),
    			resSeat: &openapi.Seat{
    				Id:     openapi.SeatID(1),
    				Status: openapi.Empty,
    			},
    			resStatus: http.StatusOK,
    		},
    		"UpdateSeatStatusの返り値に無効な座席ステータスが含まれていたら500": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.InUse,
    			},
    			seatID:                  values.SeatID(1),
    			executeUpdateSeatStatus: true,
    			seat:                    domain.NewSeat(1, 100), // 無効な座席ステータス
    			isError:                 true,
    			resStatus:               http.StatusInternalServerError,
    		},
    	}
    
    	for name, testCase := range testCases {
    		t.Run(name, func(t *testing.T) {
    			t.Parallel()
    
    			ctrl := gomock.NewController(t)
    			seatMock := mock.NewMockSeat(ctrl)
    			seatHandler := NewSeat(seatMock)
    
    			reqBody, err := json.Marshal(testCase.req)
    			assert.NoError(t, err)
    
    			req := httptest.NewRequest(http.MethodPatch, fmt.Sprintf("/seats/%d", testCase.seatID), bytes.NewBuffer(reqBody))
    			req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
    
    			rec := httptest.NewRecorder()
    			c := echo.New().NewContext(req, rec)
    
    			c.SetPath("/seats/:seatID")
    			c.SetParamNames("seatID")
    			c.SetParamValues(strconv.Itoa(int(testCase.seatID)))
    
    			if testCase.executeUpdateSeatStatus {
    				var status values.SeatStatus
    				switch testCase.req.Status {
    				case openapi.Empty:
    					status = values.SeatStatusEmpty
    				case openapi.InUse:
    					status = values.SeatStatusInUse
    				default:
    					t.Fatalf("invalid seat status: %v", testCase.req.Status)
    				}
    				seatMock.EXPECT().UpdateSeatStatus(gomock.Any(), testCase.seatID, status).
    					Return(testCase.seat, testCase.UpdateSeatStatusError)
    			}
    
    			err = seatHandler.PatchSeatStatus(c, openapi.SeatIDInPath(testCase.seatID))
    			if testCase.isError {
    				assert.Error(t, err)
    				var httpErr *echo.HTTPError
    				if errors.As(err, &httpErr) {
    					assert.Equal(t, testCase.resStatus, httpErr.Code)
    				}
    				return
    			}
    
    			assert.Equal(t, testCase.resStatus, rec.Code)
    			var res openapi.Seat
    			err = json.NewDecoder(rec.Body).Decode(&res)
    			assert.NoError(t, err)
    			assert.Equal(t, testCase.resSeat.Id, res.Id)
    			assert.Equal(t, testCase.resSeat.Status, res.Status)
    		})
    	}
    }
    Content-Typeの検証

    リクエストヘッダーのContent-Typeが期待値と異なる場合のハンドリングテストがありません。

    func TestPostSeat(t *testing.T) {
    	t.Parallel()
    
    	testCases := map[string]struct {
    		req                openapi.PostSeatRequest
    		seats              []*domain.Seat
    		UpdateSeatNumError error
    		resSeat            []*openapi.Seat
    		isError            bool
    		resStatus          int
    	}{
    		"正しく変更できる": {
    			req: openapi.PostSeatRequest{
    				Num: 3,
    			},
    			seats: []*domain.Seat{
    				domain.NewSeat(1, values.SeatStatusEmpty),
    				domain.NewSeat(2, values.SeatStatusInUse),
    				domain.NewSeat(3, values.SeatStatusEmpty),
    			},
    			resSeat: []*openapi.Seat{
    				{
    					Id:     openapi.SeatID(1),
    					Status: openapi.Empty,
    				},
    				{
    					Id:     openapi.SeatID(2),
    					Status: openapi.InUse,
    				},
    				{
    					Id:     openapi.SeatID(3),
    					Status: openapi.Empty,
    				},
    			},
    			resStatus: http.StatusOK,
    		},
    		"UpdateSeatNumがエラーなので500": {
    			req: openapi.PostSeatRequest{
    				Num: 3,
    			},
    			UpdateSeatNumError: assert.AnError,
    			isError:            true,
    			resStatus:          http.StatusInternalServerError,
    		},
    		"UpdateSeatNumで0を指定した場合": {
    			req: openapi.PostSeatRequest{
    				Num: 0,
    			},
    			seats:     []*domain.Seat{},
    			resSeat:   []*openapi.Seat{},
    			resStatus: http.StatusOK,
    		},
    	}
    
    	for name, testCase := range testCases {
    		t.Run(name, func(t *testing.T) {
    			t.Parallel()
    
    			ctrl := gomock.NewController(t)
    			seatMock := mock.NewMockSeat(ctrl)
    			seatHandler := NewSeat(seatMock)
    
    			reqBody, err := json.Marshal(testCase.req)
    			assert.NoError(t, err)
    
    			req := httptest.NewRequest(http.MethodPost, "/seats", bytes.NewBuffer(reqBody))
    			req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
    
    			rec := httptest.NewRecorder()
    			c := echo.New().NewContext(req, rec)
    
    			c.SetPath("/seats")
    
    			seatMock.EXPECT().UpdateSeatNum(gomock.Any(), uint(testCase.req.Num)).
    				Return(testCase.seats, testCase.UpdateSeatNumError)
    			err = seatHandler.PostSeat(c)
    			if testCase.isError {
    				assert.Error(t, err)
    				var httpErr *echo.HTTPError
    				if errors.As(err, &httpErr) {
    					assert.Equal(t, testCase.resStatus, httpErr.Code)
    				}
    			}
    			if err != nil {
    				return
    			}
    			assert.Equal(t, testCase.resStatus, rec.Code)
    			var res []openapi.Seat
    			err = json.NewDecoder(rec.Body).Decode(&res)
    			assert.NoError(t, err)
    			assert.Equal(t, len(testCase.resSeat), len(res))
    			for i, seat := range res {
    				assert.Equal(t, testCase.resSeat[i].Id, seat.Id)
    				assert.Equal(t, testCase.resSeat[i].Status, seat.Status)
    			}
    		})
    	}
    }
    
    func TestPatchSeatStatus(t *testing.T) {
    	t.Parallel()
    
    	testCases := map[string]struct {
    		req                     openapi.PatchSeatStatusRequest
    		seatID                  values.SeatID
    		executeUpdateSeatStatus bool
    		seat                    *domain.Seat
    		UpdateSeatStatusError   error
    		resSeat                 *openapi.Seat
    		isError                 bool
    		resStatus               int
    	}{
    		"正しく変更できる": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.InUse,
    			},
    			seatID:                  values.SeatID(1),
    			executeUpdateSeatStatus: true,
    			seat:                    domain.NewSeat(1, values.SeatStatusInUse),
    			resSeat: &openapi.Seat{
    				Id:     openapi.SeatID(1),
    				Status: openapi.InUse,
    			},
    			resStatus: http.StatusOK,
    		},
    		"無効な座席ステータスが指定されたら400": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.SeatStatus("invalid"),
    			},
    			seatID:    values.SeatID(1),
    			isError:   true,
    			resStatus: http.StatusBadRequest,
    		},
    		"UpdateSeatStatusがエラーなので500": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.InUse,
    			},
    			seatID:                  values.SeatID(1),
    			executeUpdateSeatStatus: true,
    			UpdateSeatStatusError:   assert.AnError,
    			isError:                 true,
    			resStatus:               http.StatusInternalServerError,
    		},
    		"UpdateSeatStatusで存在しない座席IDが指定されたら404": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.InUse,
    			},
    			seatID:                  values.SeatID(999),
    			executeUpdateSeatStatus: true,
    			UpdateSeatStatusError:   service.ErrNoSeat,
    			isError:                 true,
    			resStatus:               http.StatusNotFound,
    		},
    		"UpdateSeatStatusで無効な座席ステータスが指定されたら404": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.InUse,
    			},
    			seatID:                  values.SeatID(1),
    			executeUpdateSeatStatus: true,
    			UpdateSeatStatusError:   service.ErrInvalidSeatStatus,
    			isError:                 true,
    			resStatus:               http.StatusNotFound,
    		},
    		"UpdateSeatStatusで座席が空席に変更された場合": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.Empty,
    			},
    			seatID:                  values.SeatID(1),
    			executeUpdateSeatStatus: true,
    			seat:                    domain.NewSeat(1, values.SeatStatusEmpty),
    			resSeat: &openapi.Seat{
    				Id:     openapi.SeatID(1),
    				Status: openapi.Empty,
    			},
    			resStatus: http.StatusOK,
    		},
    		"UpdateSeatStatusの返り値に無効な座席ステータスが含まれていたら500": {
    			req: openapi.PatchSeatStatusRequest{
    				Status: openapi.InUse,
    			},
    			seatID:                  values.SeatID(1),
    			executeUpdateSeatStatus: true,
    			seat:                    domain.NewSeat(1, 100), // 無効な座席ステータス
    			isError:                 true,
    			resStatus:               http.StatusInternalServerError,
    		},
    	}
    
    	for name, testCase := range testCases {
    		t.Run(name, func(t *testing.T) {
    			t.Parallel()
    
    			ctrl := gomock.NewController(t)
    			seatMock := mock.NewMockSeat(ctrl)
    			seatHandler := NewSeat(seatMock)
    
    			reqBody, err := json.Marshal(testCase.req)
    			assert.NoError(t, err)
    
    			req := httptest.NewRequest(http.MethodPatch, fmt.Sprintf("/seats/%d", testCase.seatID), bytes.NewBuffer(reqBody))
    			req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
    
    			rec := httptest.NewRecorder()
    			c := echo.New().NewContext(req, rec)
    
    			c.SetPath("/seats/:seatID")
    			c.SetParamNames("seatID")
    			c.SetParamValues(strconv.Itoa(int(testCase.seatID)))
    
    			if testCase.executeUpdateSeatStatus {
    				var status values.SeatStatus
    				switch testCase.req.Status {
    				case openapi.Empty:
    					status = values.SeatStatusEmpty
    				case openapi.InUse:
    					status = values.SeatStatusInUse
    				default:
    					t.Fatalf("invalid seat status: %v", testCase.req.Status)
    				}
    				seatMock.EXPECT().UpdateSeatStatus(gomock.Any(), testCase.seatID, status).
    					Return(testCase.seat, testCase.UpdateSeatStatusError)
    			}
    
    			err = seatHandler.PatchSeatStatus(c, openapi.SeatIDInPath(testCase.seatID))
    			if testCase.isError {
    				assert.Error(t, err)
    				var httpErr *echo.HTTPError
    				if errors.As(err, &httpErr) {
    					assert.Equal(t, testCase.resStatus, httpErr.Code)
    				}
    				return
    			}
    
    			assert.Equal(t, testCase.resStatus, rec.Code)
    			var res openapi.Seat
    			err = json.NewDecoder(rec.Body).Decode(&res)
    			assert.NoError(t, err)
    			assert.Equal(t, testCase.resSeat.Id, res.Id)
    			assert.Equal(t, testCase.resSeat.Status, res.Status)
    		})
    	}
    }
    go:generate設定

    go:generateディレクティブが正しく機能するか、モックファイルの生成先やパッケージ名が合っているか確認が必要です。

    package service
    
    //go:generate go tool mockgen -typed -source=seat.go -destination=mock/seat.go -package=mock Seat
    
    import (
    	"context"
    
    	"github.com/traPtitech/trap-collection-server/src/domain"

    @github-actions
    Copy link

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @codecov
    Copy link

    codecov bot commented Jun 12, 2025

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 49.83%. Comparing base (7d65a8f) to head (93571ee).
    Report is 4 commits behind head on main.

    Additional details and impacted files
    @@            Coverage Diff             @@
    ##             main    #1255      +/-   ##
    ==========================================
    + Coverage   49.12%   49.83%   +0.70%     
    ==========================================
      Files         127      127              
      Lines       11301    11301              
    ==========================================
    + Hits         5552     5632      +80     
    + Misses       5450     5368      -82     
    - Partials      299      301       +2     

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    🚀 New features to boost your workflow:
    • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

    @ikura-hamu ikura-hamu merged commit 6e4229a into main Jun 12, 2025
    12 checks passed
    @ikura-hamu ikura-hamu deleted the test/handler_seat branch June 12, 2025 12:46
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    handler/seatのユニットテスト

    2 participants