Skip to content

Conversation

SVilgelm
Copy link
Member

@SVilgelm SVilgelm commented Sep 9, 2025

Add comprehensive for configuration parsing and port
precedence. New main_test.go covers missing config file handling,
invalid PORT env values, using env port when no flag is provided,
flag overriding env, config+flag precedence, and decoding unknown
fields. Helpers are added to write temporary config files.

Introduce a Delay field on Response (time.Duration) and apply the
delay when writing responses. Extract JSON content type into a
constant (JsonContentType) and use it when setting the header.

Simplify responsesWriter by removing the logger parameter and use
slog.Error for error reporting when writing response bodies.

Misc: update test logger helper signature to accept testing.TB and
adjust imports to include time. These changes enable testing of
port/config behavior and add configurable response delays.

@Copilot Copilot AI review requested due to automatic review settings September 9, 2025 01:12
Copy link
Contributor

@Copilot 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 the main function into a testable run() function and adds comprehensive unit tests for the CLI logic. The refactoring enables testing without actually starting the HTTP server by introducing a startServer flag parameter.

  • Extracts main logic into testable run() function with dependency injection for args, env, stderr, and server startup control
  • Adds getEnvMap() helper to capture environment variables as a map for testing
  • Introduces comprehensive unit tests covering config file handling, port precedence, and error scenarios

Reviewed Changes

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

Show a summary per file
File Description
main.go Refactors main into testable run() function with better error handling and flag parsing
main_test.go Adds unit tests for run() function covering various CLI scenarios and error cases
integration_test.go Adds integration test that starts real HTTP server and exercises endpoints
config.go Introduces JsonContentType constant to reduce string literal duplication
responses_writer_test.go Updates tests to use new JsonContentType constant instead of hardcoded strings

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@SVilgelm SVilgelm changed the title feat: refactor main and add tests feat(tests,config): add tests, response delay, and logging fixes Oct 10, 2025
@SVilgelm SVilgelm requested a review from Copilot October 10, 2025 17:03
Copy link
Contributor

@Copilot 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

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


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@SVilgelm SVilgelm force-pushed the refactor-main branch 2 times, most recently from 201552d to e25c074 Compare October 10, 2025 17:13
Add comprehensive for configuration parsing and port
precedence. New main_test.go covers missing config file handling,
invalid PORT env values, using env port when no flag is provided,
flag overriding env, config+flag precedence, and decoding unknown
fields. Helpers are added to write temporary config files.

Introduce a Delay field on Response (time.Duration) and apply the
delay when writing responses. Extract JSON content type into a
constant (JsonContentType) and use it when setting the header.

Simplify responsesWriter by removing the logger parameter and use
slog.Error for error reporting when writing response bodies.

Misc: update test logger helper signature to accept testing.TB and
adjust imports to include time. These changes enable testing of
port/config behavior and add configurable response delays.
@SVilgelm SVilgelm merged commit fd9b0c5 into main Oct 10, 2025
7 checks passed
@SVilgelm SVilgelm deleted the refactor-main branch October 10, 2025 17:16
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.

1 participant