Fix os.Exit in goroutines, Makefile macOS compat, and JSON round-trip tests#107
Open
briansumma wants to merge 2 commits into
Open
Fix os.Exit in goroutines, Makefile macOS compat, and JSON round-trip tests#107briansumma wants to merge 2 commits into
briansumma wants to merge 2 commits into
Conversation
- cmd/profile/authorize.go: Replace os.Exit(1) in goroutine with sending error to result channel, allowing proper cleanup - cmd/pullrequest/create.go: Replace fmt.Fprintf+os.Exit(1) with return err for proper error propagation through Cobra - main.go: Add comment clarifying .env load error is expected Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Makefile: - Inline test recipe to avoid $(COVERAGE_OUT) prerequisite requiring GNU Make 4 features unavailable on macOS default make - Add coverage-report target and restore GOCOV/GOCOVXML variables - Add coverage-report to .PHONY declaration - Use go install with @latest instead of deprecated go get for tools Tests: - cmd/commit: Fix MarshalJSON to omit zero-value fields, fixing round-trip assertion failures in TestCanUnmarshal - cmd/user: Fix MarshalJSON to use time pointer for omitempty - testdata/: Update branch, commit, and pullrequest fixtures to match actual Bitbucket API response structure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix
os.Exitcalls in goroutines and error paths —cmd/profile/authorize.gocalledos.Exit(1)inside a goroutine, preventing cleanup;cmd/pullrequest/create.gousedos.Exit(1)instead of returning errors through Cobra. Both now propagate errors properly.Fix Makefile
testtarget for macOS compatibility — Thetesttarget used$(COVERAGE_OUT)as a prerequisite, which relied on GNU Make 4 features unavailable in macOS defaultmake. Inlined the test recipe somake testworks out of the box. Also migrated tool installation from deprecatedgo gettogo installwith@latest.Fix JSON round-trip test failures —
MarshalJSONonCommitandUserstructs emitted zero-value fields (author: {},date: "0001-01-01...") that were absent in the original API fixtures, causingTestCanUnmarshalassertions to fail. Addedomitempty-style nil guards so marshal output matches unmarshal input.Test plan
go build ./...passesgo vet ./...passesTestPullRequestSuite/TestCanUnmarshalpassesTestPullRequestSuite/TestCanUnmarshalWithNilDestinationRepositorypassesmake teston GNU Make 4+)make coverage-reportproduces XML output