Skip to content

Add Tests#3

Open
muliswilliam wants to merge 19 commits into
mainfrom
tests
Open

Add Tests#3
muliswilliam wants to merge 19 commits into
mainfrom
tests

Conversation

@muliswilliam
Copy link
Copy Markdown
Owner

No description provided.

@muliswilliam muliswilliam requested a review from Copilot May 22, 2025 13:05
Copy link
Copy Markdown

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 PR refactors services and handlers to use interfaces, adds a comprehensive test suite for webhook handlers, and adjusts CI/Makefile for coverage reporting.

  • Introduce AuthService interface and rename concrete implementation to authService
  • Update router and handler constructors to accept service interfaces and enforce early returns on errors
  • Add handler tests and configure make coverage and CI to run tests

Reviewed Changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/service/auth.go Added AuthService interface, renamed struct to authService
internal/handlers/webhook.go Updated signature to use interfaces; added missing return after errors; removed default status fallback
cmd/server/server.go Incorrect dereference when passing request service to router
internal/metrics/recorder.go Recorder interface missing methods used in tests
internal/handlers/webhook_test.go Added extensive webhook handler test suite
Comments suppressed due to low confidence (2)

internal/handlers/webhook.go:60

  • Removing the default status fallback causes responseCode to remain 0 if the form value is empty; consider restoring if responseCode == 0 { responseCode = http.StatusOK } to preserve prior behavior.
responseCode, _ := strconv.Atoi(r.FormValue("response_code"))

internal/metrics/recorder.go:4

  • The Recorder interface lacks an IncWebhooksUpdated() method, which is invoked in handler tests; add this method (and any others tests rely on) to the interface.
type Recorder interface {

Comment thread internal/service/auth.go Outdated
Comment thread cmd/server/server.go Outdated
@muliswilliam muliswilliam requested a review from Copilot May 22, 2025 13:15
Copy link
Copy Markdown

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 PR adds test coverage for several handlers and services while refactoring service interfaces to be passed by value rather than by pointer. Key changes include updating method signatures in AuthService, modifying router function parameter types, and adding comprehensive tests for webhook handlers.

Reviewed Changes

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

Show a summary per file
File Description
internal/service/auth.go Refactored AuthService method receivers and return type
internal/routers/* Updated function signatures to use interface values
internal/repository/mocks/* Updated mocks to match new interface signatures
internal/handlers/* Updated handler constructions and error handling improvements
cmd/server/server.go, go.mod, Makefile Minor updates to imports and test-related commands
.github/workflows/ci.yaml Uncommented test step in CI workflow
Comments suppressed due to low confidence (1)

internal/handlers/webhook_api.go:21

  • The type name 'WebhookAiHandler' appears to be a typo; consider renaming it to 'WebhookApiHandler' for consistency with the file name and its usage.
type WebhookAiHandler struct {

Comment thread internal/handlers/webhook_test.go Outdated
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@muliswilliam muliswilliam requested a review from Copilot May 22, 2025 13:17
Copy link
Copy Markdown

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 PR refactors service and handler layers to depend on interfaces and adds tests for the webhook handlers.

  • Refactor constructors to accept service interfaces rather than concrete pointers
  • Add testify-based mocks and a comprehensive test suite for WebhookHandler
  • Update CI/Makefile to run tests and generate coverage

Reviewed Changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/service/auth.go Refactor AuthService to use an interface and add mockgen tag
internal/routers/webhook.go Update NewWebhookRouter signature to accept interface types
internal/routers/web.go Update NewWebRouter signature and handler argument unpacking
internal/routers/api.go Update NewApiRouter signature to accept interface types
internal/repository/mocks/webhook_mock.go Add WebhookRepositoryMock using testify
internal/repository/mocks/user_mock.go Add UserRepositoryMock using testify
internal/middlewares/middleware.go Update RequireAPIKey to accept AuthService interface
internal/metrics/mocks/recorder_mock.go Add RecorderMock for metrics.Recorder
internal/handlers/webhook_test.go Add WebhookHandler test suite using testify/suite
internal/handlers/webhook_requests.go Refactor WebhookRequestHandler to use interfaces
internal/handlers/webhook_api.go Update WebhookAiHandler to use WebhookService interface
internal/handlers/webhook.go Refactor WebhookHandler, improve error handling returns
internal/handlers/home.go Update HomeHandler to use interfaces
internal/handlers/auth.go Refactor AuthHandler to use AuthService interface
go.mod Remove duplicate testify require, add it once
cmd/server/server.go Reorder imports for routers, service, store
Makefile Add coverage target
.vscode/settings.json Disable Makefile auto-configure
.github/workflows/ci.yaml Re-enable tests step in CI
Comments suppressed due to low confidence (6)

internal/handlers/webhook_api.go:21

  • [nitpick] The struct is named WebhookAiHandler but the constructor and file refer to Api. Rename to WebhookApiHandler for consistency.
type WebhookAiHandler struct {

internal/service/auth.go:32

  • [nitpick] The comment refers to AuthService but the actual type is now authService. Update the doc comment to match the type name or adjust naming.
// AuthService holds user business logic

internal/service/auth.go:42

  • The quit channel passed to PeriodicCleanup is never closed, causing the cleanup goroutine to run indefinitely. Consider closing the channel or using a context to manage its lifecycle.
quit := make(chan struct{})

internal/handlers/webhook.go:58

  • [nitpick] Defaulting responseCode to 0 may produce an invalid HTTP status. Reintroduce a fallback (e.g., http.StatusOK) when parsing yields 0.
responseCode, _ := strconv.Atoi(r.FormValue("response_code"))

internal/middlewares/middleware.go:14

  • The RequireAPIKey middleware is untested. Add unit tests to validate behavior for valid, missing, and invalid API keys.
func RequireAPIKey(auth service.AuthService) func(http.Handler) http.Handler {

internal/handlers/webhook_requests.go:28

  • The sessionStore field is never initialized in NewWebhookRequestHandler and is not used. Remove it or initialize it to avoid dead code.
sessionStore   gormstore.Store

@muliswilliam muliswilliam requested a review from Copilot May 22, 2025 13:20
@muliswilliam muliswilliam self-assigned this May 22, 2025
Copy link
Copy Markdown

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 PR adds tests and updates service and handler signatures to use value types instead of pointers, along with improving error handling and updating CI and build configurations.

  • Refactors AuthService and other service interfaces to use non-pointer types and renames the concrete type.
  • Updates router and handler functions accordingly and adds missing return statements after error responses.
  • Includes new test cases, updated mocks, and enhancements to CI workflows and Makefile for test coverage.

Reviewed Changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/service/auth.go Refactored AuthService interface and implementation; renamed struct type.
internal/routers/*.go Converted service dependency parameters from pointers to values.
internal/repository/mocks/*.go Added/updated mocks for Webhook and User repositories.
internal/handlers/*.go Updated handlers to match new service signatures and improved error flows.
internal/handlers/webhook_test.go Added comprehensive tests for webhook endpoints.
go.mod, Makefile, .github/workflows/ci.yaml Updated dependencies and added test coverage commands in CI and Makefile.
.vscode/settings.json Added editor configuration for Makefile.
Comments suppressed due to low confidence (1)

internal/handlers/webhook_api.go:19

  • The struct name 'WebhookAiHandler' is inconsistent with the constructor 'NewWebhookApiHandler'. Consider renaming it to 'WebhookApiHandler' for consistency.
type WebhookAiHandler struct {

@muliswilliam muliswilliam requested a review from Copilot May 22, 2025 21:05
Copy link
Copy Markdown

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 PR adds comprehensive unit tests, introduces mock implementations, and standardizes handler and router constructor signatures.

  • Refactor router and handler constructors to accept service interfaces instead of pointers
  • Introduce mocks for repository, metrics, and database layers
  • Extend HTTP handlers with early returns on error and add full test coverage for WebhookHandler

Reviewed Changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/routers/webhook.go Update NewWebhookRouter to use interface types for services
internal/routers/web.go Refactor NewWebRouter signatures and fix metrics parameter
internal/routers/api.go Change NewApiRouter to use service interfaces
internal/repository/mocks/webhook_mock.go Add mock for WebhookRepository
internal/repository/mocks/user_mock.go Add mock for UserRepository
internal/middlewares/middleware.go Update RequireAPIKey to accept interface type
internal/metrics/mocks/recorder_mock.go Add mock for metrics recorder
internal/handlers/webhook_test.go Add full suite of tests for WebhookHandler
internal/handlers/webhook_requests.go Update WebhookRequestHandler to use interfaces
internal/handlers/webhook_api.go Change WebhookAiHandler (API) to use interface types
internal/handlers/webhook.go Add early returns, update logger usage, remove default code bug
internal/handlers/home.go Refactor HomeHandler constructors to use interface types
internal/handlers/auth.go Simplify AuthHandler, removed custom password validation
internal/db/mocks/db.go Add GORM DB mock connection helper
go.mod Tidy dependencies and add testify
cmd/server/server.go Update auth service initialization with hasher and validator
Makefile Add coverage target
.github/workflows/ci.yaml Enable running tests in CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants