Skip to content

Conversation

@torcolvin
Copy link
Collaborator

@torcolvin torcolvin commented Oct 10, 2025

Refactor tests in prep for CBG-4345 which changes some return values.

  • create helper functions
  • replace assert.True(t, x == y) with assert.Equal(t, x, y)
  • Create error types to assert on

This change will make the bulk of CBG-4345 less onerous to read since it will add err to several functions that are used like canSeeChannel and expandWildCardChannel and that would split cause a lot of changes to these test files.

Pre-review checklist

  • Removed debug logging (fmt.Print, log.Print, ...)
  • Logging sensitive data? Make sure it's tagged (e.g. base.UD(docID), base.MD(dbName))
  • Updated relevant information in the API specifications (such as endpoint descriptions, schemas, ...) in docs/api

Integration Tests

Refactor tests in prep for CBG-4345 which changes some
return values.

- create helper functions
- replace assert.True(t, x == y) with assert.Equal(t, x, y)
- Create error types to assert on
@torcolvin torcolvin requested review from Copilot and removed request for Copilot October 10, 2025 14:03
Copy link
Contributor

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

Refactors authentication test code to prepare for CBG-4345 by improving error handling and test assertions. Key changes include creating structured error types for authorization failures, replacing loose boolean assertions with specific error checking, and introducing helper functions to reduce code duplication.

  • Create standardized error types for different authorization failure scenarios
  • Replace assert.True(t, x == y) patterns with assert.Equal(t, x, y) for better error messages
  • Add helper functions to reduce repetitive test code and improve readability

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
auth/error.go New file defining structured error types and helper functions for authorization failures
auth/principal.go Update UnauthError interface to accept error instead of string
auth/role.go Implement new UnauthError signature and use structured errors
auth/user_collection_access.go Use new structured error types
auth/role_collection_access.go Use new structured error types and remove unused imports
db/functions.go Use structured error with HTTP status code
db/functions/function.go Use structured error with HTTP status code
auth/collection_access_test.go Add helper functions and improve test assertions
auth/auth_test.go Add helper functions and improve test assertions

gregns1
gregns1 previously approved these changes Oct 20, 2025
Copy link
Contributor

@gregns1 gregns1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM generally. Approved as can be merged as is and nit picks could be picked up in your follow up PR

@torcolvin torcolvin enabled auto-merge (squash) October 20, 2025 15:44
@torcolvin torcolvin merged commit da8b475 into main Oct 20, 2025
41 of 42 checks passed
@torcolvin torcolvin deleted the CBG-4345-err branch October 20, 2025 19:17
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.

2 participants