refactor main and improve tests

[kazeburo]

PR Type

Enhancement, Tests, Bug fix

---

Description

• Refactor main.go into focused Opt methods

• Extract request/wait loops into request.go

• Add unit tests for verify and request flows

• Fix timer leak in retry loops

---

Diagram Walkthrough

flowchart LR
  main["_main"] --> verify["opt.verify"]
  verify --> build["opt.BuildClient"]
  build --> run["opt.run"]
  run --> decision{"WaitFor?"}
  decision -- "yes" --> wait["runWaitFor"]
  decision -- "no" --> req["runRequest"]
  wait --> request["opt.Request"]
  req --> request

Loading

File Walkthrough

	Relevant files
Enhancement	4 files checker.go Use `strings.SplitSeq` and ignore `Write` errors                  +4/-4      main.go Refactor `_main` into `Opt` verify, build, and run methods +99/-93  request.go Extract request loops and fix timer cleanup                            +70/-0    writer.go Replace manual min logic with `min` builtin                            +1/-3
Tests	2 files main_test.go Add unit tests for `Opt.verify` validations                            +134/-0  request_test.go Add unit tests for request and wait loops                                +217/-0
Configuration changes	1 files Makefile Embed git commit and add lint target                                          +9/-8

---

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 16f2032)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Go version compatibility strings.SplitSeq requires Go 1.24. If the project is built with Go 1.23 or earlier, ExpectedStatusCode will fail to compile. Unless the Go version has been bumped elsewhere, this change introduces a build regression. Consider keeping strings.Split or updating go.mod to require Go 1.24. expects := strings.SplitSeq(opt.Expect, ",") for e := range expects {

[Copilot]

Pull request overview

This PR refactors the CLI entrypoint by extracting option validation and request execution into smaller methods, adds more unit tests around request/verification behavior, and enhances build metadata injection (version + commit).

Changes:

• Refactor main flow into Opt.verify(), Opt.BuildClient(), and Opt.run(), and extract request retry loops into request.go.
• Add unit tests for option verification (main_test.go) and request execution (request_test.go).
• Update build to embed Git commit metadata and simplify Makefile build targets.

Reviewed changes

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

Show a summary per file

File	Description
writer.go	Simplifies buffer-cap logic using min() when appending to the capped writer buffer.
checker.go	Updates expected-status parsing to use strings.SplitSeq and adjusts header write error handling.
main.go	Refactors validation and run flow, adds commit metadata to version output, and changes flag parser options.
request.go	New file extracting runWaitFor / runRequest loops from main.go.
request_test.go	New tests covering request outcomes (success, status mismatch, body mismatch, wait-for behavior, timeouts).
main_test.go	New tests covering Opt.verify() validation and normalization behavior.
Makefile	Adds commit ldflag, simplifies build targets to go build package mode, adds lint target.

Suppressed comments (1)

request.go:56

• Using time.After(opt.Interim) inside the retry loop allocates a fresh timer each iteration and cannot be stopped when ctx.Done() triggers, which is avoidable overhead in tight retry loops. Prefer time.NewTimer and stop/drain it on cancellation.

		select {
		case <-ctx.Done():
		case <-time.After(opt.Interim):
		}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Persistent review updated to latest commit dac0c6d

[github-actions]

Persistent review updated to latest commit 8f58720

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 8f58720

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Ensure compatibility with Go versions prior to 1.23 strings.SplitSeq requires Go 1.23+. If the project supports older versions, this will cause a compilation failure. Revert to strings.Split with a standard range loop for broader compatibility. checker.go [162-163] -expects := strings.SplitSeq(opt.Expect, ",") -for e := range expects { +expects := strings.Split(opt.Expect, ",") +for _, e := range expects { Suggestion importance[1-10]: 7 __ Why: strings.SplitSeq was introduced in Go 1.23. Using it may cause compilation failures on older versions, so reverting to strings.Split ensures broader compatibility.	Medium
	Remove redundant help output that prints The flags library already prints help output when --help is used, and err is nil in this branch. Printing err results in being output. Remove the redundant print statement. main.go [188-190] } else if flags.WroteHelp(err) { - fmt.Fprintf(os.Stdout, "%v\n", err) return OK Suggestion importance[1-10]: 5 __ Why: The flags.WroteHelp branch handles help output where err is typically nil. Printing it results in <nil> being output, so removing the print statement cleans up the output.	Low
	Explicitly handle port parsing errors to avoid silent fallback Silently ignoring strconv.Atoi errors can lead to unexpected fallback behavior. Explicitly check the error to ensure the port is only updated when parsing succeeds. main.go [89-91] if err == nil { - p, _ := strconv.Atoi(port) - opt.Port = p + p, err := strconv.Atoi(port) + if err == nil { + opt.Port = p + } } Suggestion importance[1-10]: 4 __ Why: Ignoring the error from strconv.Atoi can mask parsing issues. Explicitly handling it improves robustness, though the existing fallback to default port mitigates the risk.	Low

---

Previous suggestions

Suggestions up to commit 8f58720

Category	Suggestion	Impact
Possible issue	Reorder validation to normalize host/IP before checking options Move opt.normalizeHostAndIP() before opt.verifyHostOptions(). Currently, validation checks like SNI requirements fail if only IPAddress is provided, because normalization hasn't populated Hostname yet. main.go [111-131] func (opt *Opt) verify() error { opt.bufferSize = uint64(opt.MaxBufferSize) if err := opt.verifyWaitFor(); err != nil { return err } if err := opt.verifyExpectedContent(); err != nil { return err } + opt.normalizeHostAndIP() if err := opt.verifyHostOptions(); err != nil { return err } - - opt.normalizeHostAndIP() opt.setDefaultPort() opt.setDefaultURI() return nil } Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a logical bug where verifyHostOptions() fails if only IPAddress is provided, because normalizeHostAndIP() hasn't run yet. Fixing the order prevents false validation errors and ensures robust configuration checks.	High
General	Prevent timer leak during context cancellation Replace time.After with time.NewTimer and explicitly stop it on context cancellation. time.After creates a timer that leaks if the context cancels first, unlike the pattern correctly used in runWaitFor. request.go [57-60] +timer := time.NewTimer(opt.Interim) select { case <-ctx.Done(): -case <-time.After(opt.Interim): + if !timer.Stop() { + <-timer.C + } +case <-timer.C: } Suggestion importance[1-10]: 8 __ Why: Replacing time.After with time.NewTimer and explicitly stopping it on context cancellation prevents a resource leak, aligning with Go best practices and the pattern already correctly implemented in runWaitFor.	Medium
	Report parse errors before handling version flag Check for parse errors before evaluating opt.Version. If a user passes --version alongside invalid flags, psr.Parse() returns an error that is currently silently ignored, masking configuration mistakes. main.go [174-188] _, err := psr.Parse() +if err != nil { + fmt.Fprintf(os.Stderr, "%v\n", err) + return UNKNOWN +} if opt.Version { if commit == "" { commit = "dev" } fmt.Printf( "%s-%s\n%s/%s, %s, %s\n", filepath.Base(os.Args[0]), version, runtime.GOOS, runtime.GOARCH, runtime.Version(), commit) return OK } else if flags.WroteHelp(err) { Suggestion importance[1-10]: 7 __ Why: Checking for parse errors before evaluating opt.Version ensures that configuration mistakes are not silently ignored when --version is passed alongside invalid flags. Improves error handling flow and user feedback.	Medium

Suggestions up to commit e501ac2

Category	Suggestion	Impact
Possible issue	Fix premature return skipping default configuration Remove the premature return statement inside the if err != nil block. This early exit prevents subsequent functions like setDefaultPort() and setDefaultURI() from executing, which can leave opt.Port as 0 and opt.URI empty. main.go [76-83] if opt.IPAddress == "" { host, _, err := net.SplitHostPort(opt.Hostname) if err != nil { opt.IPAddress = opt.Hostname - return + } else { + opt.IPAddress = host } - opt.IPAddress = host } Suggestion importance[1-10]: 8 __ Why: The return statement inside normalizeHostAndIP() prematurely exits the function when net.SplitHostPort fails, which is unintended control flow. Removing it restores the original logic and ensures the function completes its normalization task correctly.	Medium

[github-actions]

Persistent review updated to latest commit b7f80de

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

Persistent review updated to latest commit 16f2032

[github-actions]

PR Code Suggestions ✨

No code suggestions found for the PR.