Style Guide #
External Style Guides #
Testing #
Our testing strategy is designed to ensure reliability, maintainability, and clarity across all code. Keep these guidelines in mind:
General Principles #
- Tests serve as guardrails for application behavior rather than proving complete correctness.
- They are not meant to validate the application under heavy concurrent usage.
- Extensive testing is crucial for reliability, but avoid redundant tests. If a scenario is already well-covered, adding tests that don’t provide new insights leads to test bloat—slowing development cycles, increasing maintenance costs, and adding excessive noise.
- Write tests that are maintainable. Avoid directly modifying database records; instead, use controlled queries to reset state when needed.
- Write the minimum amount of tests required to achieve maximum coverage. For net new code, focus on line coverage. It’s acceptable to start with fewer tests since additional requirements and bugs can be handled later without needing a major refactor of tests.
Package Organization #
- Internal Packages: Place tests in the same folder as the code to keep them close to the implementation and avoid cyclic dependencies.
- API Handlers: Locate tests for API handlers outside their packages to simulate real-world scenarios using properly injected dependencies and to prevent cyclic dependencies.
Testing Strategies #
Unit Tests #
- Use Go’s built-in testing framework for pure functions.
- When testing functions with multiple input/output scenarios, favor table-driven tests (see Table Driven Tests).
- We use the
stretchr/testifypackages:- Use
requirein place of manual error checks (i.e. instead of writingif err != nil { t.Fail... }). This immediately fails the test when a critical error is encountered. - Use
assertfor simple assertions so that tests continue running to report multiple failures. Avoid usingrequirefor trivial assertions as it causes an immediate exit. - Use
mockwhen needed to simulate dependencies.
- Use
- Instead of writing many individual
assertlines, consider using a single snapshot file when possible. Snapshots capture the complete output (asserting every field and value), which keeps test files smaller and more maintainable.
Snapshot Tests #
- Employ snapshot testing for verifying complex outputs (such as API responses or SQL queries) where manual assertions would be too verbose.
- Ensure that outputs are deterministic by controlling sources of nondeterminism (e.g., UUIDs, timestamps, unordered maps, floating-point precision).
- Avoid embedding snapshots within table-driven tests if it risks generating bloated snapshot files.
Mocking and API-Level Tests #
- Use libraries like httpmock or testify/mock to simulate external dependencies.
- For API-level tests, filter out nondeterministic headers (e.g., Date, fs-request-id, Vary) before generating snapshots.
Using Subtests for Shared Setup (New Subsection) #
We use Go’s subtests (t.Run) to group related test cases that share common, complex setup logic. This approach reduces boilerplate and improves readability.
Guiding Principle: Use a single top-level test with subtests when the setup code (e.g., initializing mocks, a logger, an HTTP server) is significantly more complex than the specific actions being tested in each sub-case.
Benefits:
- Reduces Boilerplate: Shared setup code is written only once.
- Improves Readability: Keeps related test cases logically grouped under a single parent test.
- Clear Scope: Each
t.Runcall creates a distinct scope, preventing test cases from interfering with one another.
When to Use Separate Tests:
For simple tests with minimal setup or for testing completely unrelated functionalities, separate top-level Test... functions remain appropriate.
API #
- Naming Conventions:
- Use
snake_casefor both routes and JSON property names.
- Use
- Routes:
- Use plurals for route segments.
- Example: Instead of
/api/account/{account_id}, use/api/accounts/{account_id}. This allows/api/accountsto list all accounts while accessing/api/accounts/{account_id}retrieves a single account.
- Example: Instead of
- Use plurals for route segments.
- Response Format:
- Always return responses as JSON objects.
- For a single entity:
{"ok": true, "response": {"id": "...", "name": "..."}} - For a list, nest the results to allow for future extensions (e.g., pagination):
{"ok": true, "response": {"activities": []}}
- For a single entity:
- Always return responses as JSON objects.
- IDs:
IDs should generally be UUIDs.
State #
- Avoid any form of global state to prevent race conditions and potential safety issues.
OAuth #
- Use
oauth2.Config.Clientto perform HTTP requests with OAuth. This client handles token refresh automatically.
Database (DB) #
- Naming Conventions:
- Use
snake_casefor table and column names. - Table names should be plural (e.g.,
users,tags).
- Use
- Data Modeling:
Avoid over-normalization if it complicates queries or lowers performance.
Database Column Structure #
-
Separate Columns vs. JSONB:
-
Separate Columns:
- Use for well-defined, stable fields.
- Prefer these when data is frequently used for filtering, sorting, or indexing.
- Allows enforcement of constraints (e.g.,
NOT NULL,UNIQUE,FOREIGN KEY). - Generally more efficient for updates.
-
JSONB:
- Use for highly dynamic or semi-structured data (e.g., user settings, metadata).
- Helps avoid frequent schema migrations for optional or rarely queried fields.
-
Hybrid Approach:
- Store essential fields as columns and keep optional or dynamic properties in a JSONB column (e.g., named
metadata).
- Store essential fields as columns and keep optional or dynamic properties in a JSONB column (e.g., named
-
-
Additional Guidelines:
- Timestamps:
Timestamp columns (created_at,updated_at) should use the typeTIMESTAMP WITH TIME ZONE. - Foreign Keys:
Name foreign keys following the pattern{entity}_id(e.g.,user_id,account_id).
- Timestamps:
-
Indexing Guidelines:
- Always index foreign keys.
- Use GIN indexes for JSONB fields when queries on them are frequent:
CREATE INDEX idx_metadata ON notes USING GIN (metadata) - Avoid indexes on JSONB fields when structured columns suffice.
Query Building: goqu vs. Raw SQL #
-
goqu:
Use goqu (a minimal ORM) for constructing queries with reusable components, especially when queries are simple with many variable parameters.Example:
func (account Account) selectDataset(where ...exp.Expression) *goqu.SelectDataset { q := goqu. From(EntityAccountsTable). Select( "fs_id", "account_id", "domain", "name", "properties", "traits", "created_at", "updated_at", "enriched_at", "deleted_at", "present_at", "prediction", "prediction_score", "amount", "expires_at", "churned", "churned_at", ) if len(where) > 0 { q = q.Where(where...) } return q } -
Raw SQL:
For more complex queries, writing raw SQL is preferred as it is generally easier to read, understand, and execute directly for debugging purposes.
Return Values and Error Handling #
-
Entity Not Found: Return
nil, nilrather than an empty struct or a custom error to clearly indicate that a requested entity was not found. -
Error Wrapping and Context: Always wrap errors to create a stack of context using the
fmt.Errorf("...: %w", err)pattern.- Be Concise: The added context should describe the operation that failed, not the error itself. For example:
fmt.Errorf("unmarshal response: %w", err). - Avoid Redundancy: Omit phrases like “failed to” or “error occurred” in error messages.
- Keep Details in Logs: Error messages should not contain verbose details like full HTTP response payloads or complex state information. That level of detail belongs in structured logs. Errors may bubble up to user-facing systems, so they must be kept clean and concise.
- Be Concise: The added context should describe the operation that failed, not the error itself. For example:
-
Resource Cleanup:
- Always use
defer rows.Close()after processing database rows. - Check
rows.Err()after row processing and handlectx.Err()appropriately during row reading.
- Always use
Integration Client Design & Testing #
When building a client package to communicate with a third-party API (e.g., Salesforce, JIRA), we follow a standard architectural pattern to ensure decoupling, clarity, and robust testability.
Guiding Principle: The Consumer Defines the Contract #
The core principle is that the consumer of a client library defines the specific interface it requires. This decouples the business logic from the concrete implementation of the API client, allowing for simple, reliable mocking using stretchr/testify/mock.
This pattern consists of three parts:
- The Concrete Client: A package (e.g.,
integrations/salesforce) that handles all direct API communication, including HTTP transport, authentication, and (de)serialization. - The Consumer: A package (e.g.,
connections) that orchestrates business logic and uses the concrete client. - The Interface: An interface defined inside the consumer package that specifies only the client methods the consumer needs.
The Pattern in Practice #
This example uses the SalesforceConnection as the standard.
-
The Concrete Client (
integrations/salesforce): This package contains theClientstruct and its methods.// In integrations/salesforce/client.go package salesforce type Client struct { /* ... */ } func (c *Client) GetOpportunities(ctx context.Context) ([]Opportunity, error) { /* ... */ } func (c *Client) Query(ctx context.-Context, q string) (QueryResult, error) { /* ... */ } -
The Consumer-Side Interface (
connections): TheSalesforceConnectiondefines thesalesforceClientinterface it depends on.// In connections/salesforce.go package connections type salesforceClient interface { GetOpportunities(context.Context) ([]salesforce.Opportunity, error) Query(context.Context, string) (salesforce.QueryResult, error) } type SalesforceConnection struct { client salesforceClient // ... } -
Testing with
testify/mock(connections): In the consumer’s tests, usetestify/mockto create a mock implementation of thesalesforceClientinterface. You should never need to mock the HTTP transport layer.// In connections/salesforce_test.go package connections import ( "testing" "github.com/stretchr/testify/mock" ) // mockSalesforceClient embeds mock.Mock to get mocking capabilities. type mockSalesforceClient struct { mock.Mock } // GetOpportunities is the mock implementation. func (m *mockSalesforceClient) GetOpportunities(ctx context.Context) ([]salesforce.Opportunity, error) { args := m.Called(ctx) // Type assert the return values from the mock setup. if opps, ok := args.Get(0).([]salesforce.Opportunity); ok { return opps, args.Error(1) } return nil, args.Error(1) } // Query is the other mock implementation. func (m *mockSalesforceClient) Query(ctx context.Context, q string) (salesforce.QueryResult, error) { args := m.Called(ctx, q) if res, ok := args.Get(0).(salesforce.QueryResult); ok { return res, args.Error(1) } return salesforce.QueryResult{}, args.Error(1) } func TestSalesforceConnection_Sync(t *testing.T) { // 1. Create an instance of the mock. mockClient := new(mockSalesforceClient) // 2. Set up expectations. mockOpportunities := []salesforce.Opportunity{{ID: "opp-1"}} mockClient.On("GetOpportunities", mock.Anything).Return(mockOpportunities, nil) // 3. Inject the mock into the consumer. conn := &SalesforceConnection{client: mockClient, ...} // 4. Run the test. err := conn.Sync(context.Background()) // ... assertions // 5. Assert that the expectations were met. mockClient.AssertExpectations(t) }
Handling Rate Limits #
Rate limiting is a responsibility of the concrete client package. It should be handled transparently to the consumer whenever possible.
- Implement retry logic, preferably with exponential backoff, within a shared request function inside the client (e.g., a private
doRequesthelper). - After a reasonable number of retries, if the request is still failing due to rate limits, the client should return a specific, identifiable rate limit error.
- This allows the consumer (e.g., a
Connectionor background job) to catch the specific error and decide on a more intelligent course of action, such as rescheduling the operation, rather than simply failing.
Docker #
- Ensure the build process is self-contained within a Dockerfile; all builds and dependencies must occur inside the Dockerfile—no external tools are permitted.
- Container images must support multiple architectures (multi-arch) as our infrastructure includes both x86 and ARM devices/instances.
Goroutines #
- Use goroutines and channels judiciously.
Since the Go web server handles each request in its own goroutine, introduce additional concurrency only when absolutely necessary to avoid complicating production environments.
Internal Data Modeling & API Entity Resolution #
This section outlines how we model data within our internal services and how we handle the resolution of entity references, particularly for API responses. The goal is to maintain a clean, efficient internal architecture while providing rich, user-friendly API outputs.
Guiding Principle: Lean Internal Models, Rich API Responses #
- Internal Simplicity: Foreign key references should be stored and handled using their raw ID form (e.g.,
user_idas a*stringUUID) within internal packages (DAOs, services). This approach keeps internal models lean, closely aligned with the database schema, and helps avoid unnecessary data fetching or complex object graph management in core business logic. - Enhanced Decoupling: This method enhances decoupling. Internal services can operate without tight coupling to the specific presentation needs of API clients or the full structure of related tenant-specific entities (like
tenant.User) when only an ID is required for internal logic.
User Entity Resolution for APIs (e.g., created_by, updated_by)
#
A common and critical application of this principle is the handling of user references, such as created_by and updated_by fields, which point to entries in the users table.
- Storage & Internal Handling: These user reference fields are stored as simple
*stringUUIDs in the database. They should be handled as these raw IDs within and between internal services and DAOs. - API Presentation: For API responses, these user IDs must be resolved into richer object representations (e.g.,
created_by: {"id": "user-uuid", "name": "User Name", ...}). This provides clients with immediately useful information. - Resolution Point: This resolution process is the responsibility of the API layer. It should occur, for example, within API handlers or dedicated response formatting/assembly components, just before the response is sent to the client.
- Rationale for this Specific Focus:
- Improved Client Experience: API consumers receive comprehensive user information directly, reducing the need for them to make subsequent lookups.
- Internal Efficiency: We avoid burdening numerous internal operations with the cost (database queries, object assembly) and complexity of fetching and managing full
tenant.Userobjects when only the ID is needed for internal logic or database relations. - Simplified DAOs: DAOs remain focused on their primary entities and are not responsible for resolving relationships that are only needed for presentation.
General Entity Resolution in Higher Layers #
- The principle of resolving IDs primarily in higher layers (like the API layer) applies more broadly when full entity details are needed for external presentation, not just for user IDs.
- For efficient resolution, especially with lists of items, consider strategies like batching queries (e.g., collecting all unique user IDs from a list of records and fetching them in a single query) or using caching mechanisms.
Overall Benefits of this Approach #
- Performance: Reduces database load and processing overhead in background tasks and internal operations by fetching full entity details only when explicitly required for an external-facing response.
- Clarity & Simplicity: Internal code remains focused on essential data IDs, making it easier to understand and maintain. Direct mapping to the database schema for IDs is generally more efficient for internal operations.
- Reduced Coupling: Internal modules are less likely to require imports or have direct dependencies on the full structure of potentially complex related entities if they only operate on IDs, aiding modularity and reducing the ripple effect of changes.
Trade-offs #
- The API layer takes on the explicit responsibility of resolving these IDs and assembling richer objects for responses. This is a conscious architectural choice that prioritizes internal system efficiency and decoupling.
Conventional Commits #
We follow the Conventional Commits specification using the following commit types:
- fix: Bug fixes.
- chore: Non-functional changes (e.g., build process adjustments, documentation updates).
- refactor: Code refactoring that does not add features or fix bugs.
- feat: New features.
- ci: Changes to CI configuration and scripts.
- style: Code style adjustments (e.g., formatting, missing semicolons) that do not affect functionality.
- perf: Performance improvements.
- test: Adding or correcting tests.
Additional Guidelines:
- Scopes are optional but recommended, and can refer to a package name or a broader feature.
Example:feat(api)!: send an email when a product is shipped - The commit message may include an optional body and footer for additional context or issue references.
Branch Naming #
Use a consistent and descriptive branch naming convention to keep the repository organized.
Format #
<type>/<issueID>-<short-description>
Types #
Use the same type prefixes defined by the Conventional Commits specification (see the reference list) and pick the most specific option for your branch.
Guidelines #
- Always include the numeric GitHub issue ID if available.
- If no issue exists, coordinate with the team before creating a new branch.
- Write the issue description in kebab-case (all lowercase, words separated by hyphens).
Example #
feat/1234-add-customer-sync-job