Refactor: align codebase with engineering standards#96
Merged
Conversation
- Add STANDARDS.md to repo - Add revive, gosec, errorlint, exhaustive linters to .golangci.yml - Add `tidy` and `check` targets to Makefile - Fix errorlint: replace type assertions with errors.As in errors.IsRetryable() and initcmd.errorAs (bug fixes - wrapped errors were silently missed) - Fix gosec: add nolint annotations for legitimate file operations - Fix revive: rename unused parameters to _, fix labelIds -> labelIDs - Add temporary lint exclusions for issues addressed by later commits (package comments, interface stuttering, mock comments)
Create internal/testutil/assert.go with generic test helpers that replace testify assertions: Equal, NoError, Error, ErrorIs, Contains, NotContains, Len, Nil, NotNil, True, False, Empty, NotEmpty, Greater, GreaterOrEqual, Less. All helpers use t.Helper() for accurate failure line reporting and accept testing.TB for flexibility. Includes comprehensive tests.
Migrate all 44 test files from testify/assert and testify/require to either the internal testutil package or raw stdlib assertions. Packages that would create import cycles (auth, config, keychain, log, gmail, calendar, contacts, drive) use stdlib directly. Adds SliceContains and LenSlice helpers to testutil. Fixes Nil helper to correctly handle nil slices/pointers boxed into interface values using reflection. Removes github.com/stretchr/testify and all transitive dependencies (go-spew, go-difflib, yaml.v3) from go.mod.
Replace all "failed to X" error prefixes with present participle form
("Xing") across 40 source and test files (149 replacements). This
follows Go error string conventions where errors describe what was
happening when the failure occurred, not that it failed.
Move interface definitions to the packages that consume them per Go best practices. Each cmd/ package now owns its interface (MailClient, CalendarClient, ContactsClient, DriveClient) and its mock. This removes the coupling between implementation and consumer packages and eliminates the centralized testutil/mocks.go file.
All command handlers now extract context from the cobra command and pass it through to client constructors. This enables proper cancellation via signal handling. The ClientFactory signature now accepts context.Context.
Main now creates a signal-aware context (SIGINT, SIGTERM) and passes it through ExecuteContext, enabling graceful cancellation of in-flight API calls when the user presses Ctrl+C.
Callers now use auth.X() directly instead of gmail.X() wrappers. Removes GetConfigDir, GetCredentialsPath, GetOAuthConfig, ExchangeAuthCode, GetAuthURL, and ShortenPath from the gmail package.
Add package-level doc comments to all 15 packages that were missing them. Fix lint issues: goimports ordering, unused parameters, exhaustive switch, and unused struct field.
Add parallel markers to 18 test files that don't use shared mutable state (ClientFactory, os.Stdout, os.Chdir). Subtests using t.Setenv correctly omit t.Parallel() since the two are incompatible.
Thread context.Context through all API client methods (gmail, calendar, contacts, drive) so cancellation and deadlines propagate to Google API calls. Fix error wrapping in NewClient constructors to follow standards. Update PersistentTokenSource to accept context instead of using context.Background(). Align Makefile check target to use tidy instead of fmt, and update CI Go version from 1.21 to 1.24 to match go.mod.
…docs Add harness engineering infrastructure to mechanically enforce codebase conventions and improve agent legibility: - Structural tests (internal/architecture/) enforce client interfaces, ClientFactory, NewCommand(), --json flags, dependency direction, and read-only scopes across all domain packages - Centralize captureOutput/withMockClient/withFailingClientFactory into testutil.CaptureStdout and testutil.WithFactory[T] generics - Restructure AGENT.md (338->93 lines) into progressive-disclosure docs/ with architecture.md, golden-principles.md, and adding-a-domain.md - Clean up stale lint suppressions in .golangci.yml - Add coverage gate (60% floor) to Makefile and CI
All list/search commands now output [] instead of plaintext "No X found" messages when --json is active and there are no results. This fixes JSON piping (e.g., `gro mail search "..." --json | jq ...`) which previously failed with a parse error on empty result sets. Affected commands: mail search, mail thread, mail labels, mail attachments list, calendar events/today/week, calendar list, contacts list/search/groups, drive list/search/drives. Adds 12 unit tests covering the empty-results-with-JSON path for each affected command.
rianjs
changed the title
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
Closes #97
Comprehensive refactoring to align the gro codebase with engineering standards for consistency, mechanical enforcement, and maintainability.
Architecture & Enforcement
internal/architecture/architecture_test.go) enforcing codebase patterns at CI timeCode Standards
context.Contextthrough all API client methods and command handlersTest Infrastructure
CaptureStdoutand genericWithFactory[T]helperst.Parallel()to safe test filesDocumentation
docs/architecture.md,docs/golden-principles.md,docs/adding-a-domain.mdBug Fixes
--jsonflag ignored on empty results across all list/search commandsBuild & Lint
Test plan
make checkpasses (tidy, lint, test, build)make test-cover-checkpasses (63.6% > 60% threshold)golangci-lintreports 0 issues