-
Notifications
You must be signed in to change notification settings - Fork 39
Migrate test cleanup functions to use Go's built-in t.Cleanup mechanism across test utilities and test files #789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
## Walkthrough
This change refactors test setup and teardown across multiple packages by removing explicit cleanup functions and defers. Instead, cleanup logic is now registered directly within helper functions using the testing framework's `t.Cleanup` mechanism. This affects test utilities, blockchain, API, indexer, and related test files, simplifying resource management and function signatures.
## Changes
| Files/Group | Change Summary |
|-------------------------------------------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| pkg/testutils/anvil/anvil.go, pkg/testutils/anvil/anvil_test.go | `StartAnvil` no longer returns a cleanup function; resource cleanup is registered with `t.Cleanup`. Tests remove cleanup handling and delete the cleanup verification test. |
| pkg/testutils/api/api.go | API client/server creation helpers no longer return cleanup functions or accept context parameters; cleanup is handled internally via `t.Cleanup`. Function signatures updated accordingly. |
| pkg/api/message/newestEnvelope_test.go, pkg/api/message/publish_test.go, pkg/api/query_test.go | Tests remove capture and invocation of cleanup functions from API client helpers; only necessary values are assigned. |
| pkg/api/message/subscribe_test.go, pkg/api/metadata/cursor_test.go | Test setup helpers' signatures changed to omit cleanup return; all call sites updated to remove deferred cleanup calls. |
| pkg/api/payer/clientManager_test.go, pkg/api/payer/publish_test.go, pkg/api/payer/service_test.go | Tests and helpers updated to remove explicit cleanup returns and defers; resource cleanup registered with `t.Cleanup` where needed. Helper signatures updated. |
| pkg/blockchain/blockchainPublisher_test.go, pkg/blockchain/migrator/migrator_test.go | Helper functions for test setup no longer return cleanup functions; cleanup handled with `t.Cleanup`. Tests updated to remove deferred cleanup calls and variables. |
| pkg/blockchain/ratesAdmin_test.go, pkg/blockchain/registryCaller_test.go | Tests and helpers updated to remove explicit cleanup handling for Anvil or registry helpers; cleanup now handled internally or omitted. |
| pkg/blockchain/registryAdmin_test.go | `buildRegistry` signature changed to omit cleanup return; context cancellation registered with `t.Cleanup`. Tests updated to remove deferred cleanup. |
| pkg/blockchain/rpcLogStreamer_test.go | Test updated to remove explicit cleanup for Anvil instance; only RPC URL is captured. |
| pkg/indexer/e2e_test.go, pkg/indexer/storer/groupMessage_test.go, pkg/indexer/storer/identityUpdate_test.go | Helper functions and tests updated to remove cleanup returns and deferred calls; context cancellation and resource cleanup handled with `t.Cleanup`. |
| pkg/server/server_test.go | API client creation in tests updated to remove context and cleanup handling; only server address is used. |
| pkg/testutils/contracts_test.go | Tests updated to remove cleanup handling for Anvil; only RPC URL is captured. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant Test
participant HelperFunction
participant Resource
Test->>HelperFunction: Call setup helper (e.g., StartAnvil, buildPublisher)
HelperFunction->>Resource: Allocate resource (e.g., start server/process)
HelperFunction->>Test: Return resource handle(s)
HelperFunction->>HelperFunction: Register cleanup with t.Cleanup
Test->>Resource: Use resource in test
Note over HelperFunction,Resource: Cleanup is triggered automatically after test via t.CleanupPossibly related PRs
Suggested reviewers
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/testutils/api/api.go (1)
57-74: Consider refactoring NewPayerAPIClient to use t.Cleanup for consistency.While other API client creation functions have been refactored to use
t.Cleanup,NewPayerAPIClientstill returns a cleanup function. For consistency, consider updating this function to match the pattern used inNewReplicationAPIClientandNewMetadataAPIClient.-func NewPayerAPIClient( - t *testing.T, - ctx context.Context, - addr string, -) (payer_api.PayerApiClient, func()) { +func NewPayerAPIClient( + t *testing.T, + addr string, +) payer_api.PayerApiClient { dialAddr := fmt.Sprintf("passthrough://localhost/%s", addr) conn, err := grpc.NewClient( dialAddr, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithDefaultCallOptions(), ) require.NoError(t, err) client := payer_api.NewPayerApiClient(conn) - return client, func() { - err := conn.Close() - require.NoError(t, err) - } + t.Cleanup(func() { + require.NoError(t, conn.Close()) + }) + return client }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
pkg/api/message/newestEnvelope_test.go(1 hunks)pkg/api/message/publish_test.go(11 hunks)pkg/api/message/subscribe_test.go(9 hunks)pkg/api/metadata/cursor_test.go(4 hunks)pkg/api/payer/clientManager_test.go(1 hunks)pkg/api/payer/publish_test.go(3 hunks)pkg/api/payer/service_test.go(1 hunks)pkg/api/query_test.go(10 hunks)pkg/blockchain/blockchainPublisher_test.go(5 hunks)pkg/blockchain/migrator/migrator_test.go(7 hunks)pkg/blockchain/ratesAdmin_test.go(1 hunks)pkg/blockchain/registryAdmin_test.go(5 hunks)pkg/blockchain/registryCaller_test.go(3 hunks)pkg/blockchain/rpcLogStreamer_test.go(1 hunks)pkg/indexer/e2e_test.go(3 hunks)pkg/indexer/storer/groupMessage_test.go(4 hunks)pkg/indexer/storer/identityUpdate_test.go(3 hunks)pkg/server/server_test.go(2 hunks)pkg/testutils/anvil/anvil.go(2 hunks)pkg/testutils/anvil/anvil_test.go(2 hunks)pkg/testutils/api/api.go(6 hunks)pkg/testutils/contracts_test.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (16)
pkg/blockchain/rpcLogStreamer_test.go (1)
pkg/testutils/anvil/anvil.go (1)
StartAnvil(48-77)
pkg/api/message/newestEnvelope_test.go (1)
pkg/testutils/api/api.go (1)
NewTestReplicationAPIClient(213-219)
pkg/blockchain/ratesAdmin_test.go (1)
pkg/testutils/anvil/anvil.go (1)
StartAnvil(48-77)
pkg/testutils/contracts_test.go (2)
pkg/testutils/anvil/anvil.go (1)
StartAnvil(48-77)pkg/testutils/contracts.go (1)
DeployNodesContract(178-180)
pkg/api/message/publish_test.go (1)
pkg/testutils/api/api.go (1)
NewTestReplicationAPIClient(213-219)
pkg/server/server_test.go (1)
pkg/testutils/api/api.go (1)
NewReplicationAPIClient(38-55)
pkg/testutils/anvil/anvil_test.go (1)
pkg/testutils/anvil/anvil.go (1)
StartAnvil(48-77)
pkg/api/payer/clientManager_test.go (1)
pkg/testutils/api/api.go (1)
NewTestAPIServer(100-211)
pkg/blockchain/registryAdmin_test.go (4)
pkg/blockchain/registryAdmin.go (1)
INodeRegistryAdmin(20-34)pkg/blockchain/registryCaller.go (1)
INodeRegistryCaller(15-19)pkg/testutils/log.go (1)
NewLog(17-25)pkg/testutils/anvil/anvil.go (1)
StartAnvil(48-77)
pkg/indexer/storer/groupMessage_test.go (4)
pkg/indexer/storer/groupMessage.go (1)
GroupMessageStorer(23-27)pkg/testutils/store.go (1)
NewDB(65-70)pkg/db/queries/db.go (1)
New(19-21)pkg/testutils/anvil/anvil.go (1)
StartAnvil(48-77)
pkg/indexer/e2e_test.go (4)
pkg/db/queries/db.go (2)
Queries(23-25)New(19-21)pkg/testutils/log.go (1)
NewLog(17-25)pkg/testutils/store.go (1)
NewDB(65-70)pkg/testutils/anvil/anvil.go (1)
StartAnvil(48-77)
pkg/api/query_test.go (1)
pkg/testutils/api/api.go (1)
NewTestReplicationAPIClient(213-219)
pkg/indexer/storer/identityUpdate_test.go (4)
pkg/indexer/storer/identityUpdate.go (1)
IdentityUpdateStorer(32-37)pkg/mocks/mlsvalidate/mock_MLSValidationService.go (1)
MockMLSValidationService(19-21)pkg/testutils/store.go (1)
NewDB(65-70)pkg/testutils/anvil/anvil.go (1)
StartAnvil(48-77)
pkg/api/message/subscribe_test.go (1)
pkg/testutils/api/api.go (2)
ApiServerMocks(94-98)NewTestReplicationAPIClient(213-219)
pkg/blockchain/migrator/migrator_test.go (4)
pkg/blockchain/registryAdmin.go (1)
INodeRegistryAdmin(20-34)pkg/blockchain/registryCaller.go (1)
INodeRegistryCaller(15-19)pkg/testutils/log.go (1)
NewLog(17-25)pkg/testutils/anvil/anvil.go (1)
StartAnvil(48-77)
pkg/testutils/api/api.go (1)
pkg/api/server.go (1)
ApiServer(29-36)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Code Review
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd-cli)
- GitHub Check: Push Docker Images to GitHub Packages (xmtpd)
- GitHub Check: Test (Node)
- GitHub Check: Upgrade Tests
🔇 Additional comments (78)
pkg/api/payer/service_test.go (1)
68-68: Code cleanup improvement - good!The change removes the explicit cleanup function return value from
buildPayerService, which is part of a broader refactoring pattern across the codebase. Instead of requiring callers to explicitly defer cleanup functions, the cleanup is now registered directly witht.Cleanup()in the helper function, which simplifies test code and ensures proper resource cleanup.pkg/blockchain/registryCaller_test.go (4)
10-10: Simplified test cleanup - LGTMThe signature of
buildRegistryfunction has been updated to no longer return a cleanup function. This is part of a codebase-wide refactoring to register cleanup handlers witht.Cleanup()inside the helper functions rather than requiring explicit deferred cleanup calls in tests.
20-20: Consistent change pattern - LGTMThe removal of the cleanup function return value from
buildRegistryhas been applied consistently throughout all test functions, making the test code cleaner and less prone to cleanup errors.
30-30: Good refactoring consistencyThe pattern of removing explicit cleanup functions is consistently applied here, simplifying the test code while still ensuring proper resource cleanup through
t.Cleanup().
37-37: Refactoring completed correctlyThe change is consistent with the overall refactoring pattern, removing the cleanup return value while preserving the necessary test functionality.
pkg/blockchain/ratesAdmin_test.go (1)
19-19: Improved cleanup management - LGTMThe
StartAnvilfunction no longer returns a cleanup function that needs to be deferred. Instead, as shown in the relevant code snippet, it now registers the cleanup directly witht.Cleanup()internally:t.Cleanup(func() { _ = cmd.Process.Kill() _ = cmd.Wait() })This is a good improvement to simplify test resource management.
pkg/api/message/publish_test.go (2)
25-25: Simplified test cleanup pattern - LGTMThe test helper function
NewTestReplicationAPIClienthas been updated to no longer return a cleanup function. Instead, cleanup is now handled internally viat.Cleanup(), which simplifies the test code by removing the need for explicit deferred cleanup calls.
81-81: Consistent cleanup simplification applied throughoutThe pattern of removing explicit cleanup functions has been consistently applied throughout all test functions in this file. This is part of a codebase-wide refactoring to simplify resource management by embedding cleanup logic within the test lifecycle using
t.Cleanup().Also applies to: 98-98, 121-121, 138-138, 158-158, 202-202, 241-241, 281-281, 298-298, 342-342
pkg/blockchain/rpcLogStreamer_test.go (1)
48-48: Clean refactoring of resource managementThe code now relies on
t.Cleanupregistration within theStartAnvilfunction instead of explicitly handling the cleanup function in the test. This is consistent with the broader cleanup simplification across the codebase.pkg/testutils/contracts_test.go (2)
12-12: Simplified resource cleanup patternThe refactoring removes the explicit cleanup function return value and deferred cleanup call, relying instead on internal cleanup registration in the
StartAnvilfunction. This improves code clarity and reduces boilerplate.
18-18: Consistent cleanup approach appliedThis change follows the same pattern of relying on automatic cleanup registration, making the test code more concise and less error-prone.
pkg/api/message/newestEnvelope_test.go (1)
64-64: Streamlined API client creationThe code now captures only three return values from
NewTestReplicationAPIClient, discarding the cleanup function that was previously returned. Resource cleanup is now handled internally viat.Cleanupin the helper function, eliminating the need for explicit cleanup handling in tests.pkg/api/query_test.go (2)
84-84: Standardized cleanup managementThe test now relies on automated cleanup registration within
NewTestReplicationAPIClientinstead of explicitly handling cleanup. This simplifies test code and avoids potential resource leaks.
99-99: Consistent cleanup pattern applied throughout the fileAll test cases have been refactored to follow the same pattern of omitting explicit cleanup functions, relying instead on the testing framework's built-in cleanup mechanism. This improves code consistency and maintainability.
Also applies to: 114-114, 132-132, 150-150, 167-167, 187-187, 207-207, 227-227, 244-244
pkg/server/server_test.go (2)
140-141: Cleanup management simplified through internal t.CleanupThe code now uses the testing framework's built-in cleanup mechanism instead of returning and deferring explicit cleanup functions. This is a good improvement as it reduces boilerplate and makes tests less prone to resource leaks.
251-251: Clean API client creation without explicit cleanupThis change is consistent with the pattern applied throughout the codebase, removing the need to manage cleanup functions manually and instead relying on the testing framework's automatic resource management.
pkg/api/payer/clientManager_test.go (1)
25-26: Test server creation simplified with implicit cleanupThe test server creation no longer returns cleanup functions that need to be manually deferred. Instead, cleanup is registered internally with
t.Cleanup()in theNewTestAPIServerfunction, as seen in the provided snippet frompkg/testutils/api/api.go. This makes the test code cleaner and more maintainable.pkg/api/metadata/cursor_test.go (4)
28-31: Helper function signature simplified by removing cleanup functionThe
setupTestfunction no longer returns a cleanup function, aligning with the pattern of usingt.Cleanup()within helper functions. The function signature is cleaner and the responsibility for cleanup is correctly handled by the test utilities.
89-89: Return statement simplified without cleanup functionThis change completes the refactoring of the
setupTestfunction by removing the cleanup function from the return values.
106-106: Test setup call no longer needs to defer cleanupThe test now correctly relies on the internal cleanup mechanism registered by
NewTestMetadataAPIClient, removing the need for explicit cleanup handling.
146-146: Consistent cleanup approach in all test functionsLike the previous test function, this one also benefits from the simplified setup without manual cleanup handling.
pkg/indexer/storer/groupMessage_test.go (6)
21-24: Refactored helper function with improved cleanup managementThe
buildGroupMessageStorerfunction now registers context cancellation directly witht.Cleanup()rather than returning a cleanup function. This is a cleaner approach that ensures resources are properly released even if tests fail.
26-26: Anvil test environment with internal cleanupThe call to
anvil.StartAnvilno longer requires capturing a cleanup function since the function now handles cleanup internally usingt.Cleanup(). According to the relevant snippet, this ensures the Anvil process is properly terminated when the test completes.
43-43: Simplified return statement without cleanup functionThe function now returns only the storer instance since cleanup is handled internally via the testing framework.
48-48: Test call without explicit cleanup handlingThe test now uses the simplified helper function which internally registers cleanup tasks, removing the need for deferred cleanup calls in the test function itself.
89-89: Consistent use of internal cleanup in another test functionThis test also benefits from the simplified helper function with internal cleanup registration.
123-123: Consistent cleanup approach across all test functionsThe same pattern is applied throughout the test file, ensuring a consistent approach to resource management.
pkg/testutils/anvil/anvil.go (2)
47-48: Function signature simplified to use Go's testing cleanup feature.The function signature now correctly reflects that
StartAnvilreturns only the URL string, removing the explicit cleanup function return.
71-76: Good use of the t.Cleanup pattern.This change leverages Go's testing framework's built-in cleanup mechanism. Using
t.Cleanupensures that resources are properly released when tests complete, even if they fail or panic. This approach is more maintainable and less error-prone than returning explicit cleanup functions.pkg/testutils/anvil/anvil_test.go (3)
12-12: Test code simplified by removing manual cleanup handling.The test now correctly uses the updated
StartAnvilfunction which internally registers cleanup witht.Cleanup.
26-29: Struct simplified by removing unnecessary cleanup field.Cleanup is now handled by the testing framework via
t.Cleanupin theStartAnvilfunction, so the struct no longer needs to store the cleanup function.
35-37: Simplified concurrent test setup by eliminating manual cleanup.This change reflects the updated
StartAnvilfunction signature and reduces the complexity of concurrent test management by removing manual cleanup handling.pkg/blockchain/registryAdmin_test.go (10)
14-16: Simplified function signature by removing explicit cleanup return.The function signature has been updated to remove the explicit cleanup function return, aligning with the pattern of using
t.Cleanupfor resource management.
18-18: Added proper context cancellation using t.Cleanup.This change ensures that the context is properly cancelled when the test completes, preventing potential resource leaks.
20-20: Updated StartAnvil usage to match new signature.The code now correctly uses the updated
StartAnvilfunction which no longer returns a cleanup function.
41-42: Simplified return by removing explicit cleanup function.The function now returns only the registry, caller, and context, as cleanup is handled internally via
t.Cleanup.
67-68: Updated function call to match new buildRegistry signature.This test now correctly uses the updated
buildRegistryfunction signature, removing the need for explicit cleanup handling.
84-85: Updated function call to match new buildRegistry signature.Test properly updated to use the simplified function signature.
90-91: Updated function call to match new buildRegistry signature.Test properly updated to use the simplified function signature.
100-101: Updated function call to match new buildRegistry signature.Test properly updated to use the simplified function signature.
107-108: Updated function call to match new buildRegistry signature.Test properly updated to use the simplified function signature.
114-115: Updated function call to match new buildRegistry signature.Test properly updated to use the simplified function signature.
pkg/blockchain/migrator/migrator_test.go (9)
17-19: Simplified function signature by removing explicit cleanup return.The function signature has been updated to remove the explicit cleanup function return, aligning with the pattern of using
t.Cleanupfor resource management.
21-23: Added proper context cancellation using t.Cleanup.This change ensures that the context is properly cancelled when the test completes, preventing potential resource leaks.
25-25: Updated StartAnvil usage to match new signature.The code now correctly uses the updated
StartAnvilfunction which no longer returns a cleanup function.
36-38: Added client cleanup using t.Cleanup.This change ensures that the blockchain client is properly closed when the test completes, preventing potential resource leaks from unclosed connections.
56-57: Simplified return by removing explicit cleanup function.The function now returns only the registry objects, as cleanup is handled internally via
t.Cleanup. This simplifies the interface and reduces the burden on callers.
80-81: Updated function call to match new setupRegistry signature.Test properly updated to use the simplified function signature.
103-104: Updated function call to match new setupRegistry signature.Test properly updated to use the simplified function signature.
125-126: Updated function call to match new setupRegistry signature.Test properly updated to use the simplified function signature.
134-135: Updated function call to match new setupRegistry signature.Test properly updated to use the simplified function signature.
pkg/blockchain/blockchainPublisher_test.go (7)
15-17: Clean and improved test helper signatureThe function now uses
t.Cleanup(cancel)to automatically handle context cancellation rather than returning a cleanup function. This makes the API simpler and reduces the risk of forgetting to call cleanup functions.
19-19: Proper usage of the updated anvil.StartAnvilThe function correctly uses the updated
anvil.StartAnvilwhich now handles its own cleanup throught.Cleanup()internally, simplifying this code.
40-42: Good resource management with t.CleanupUsing
t.Cleanupto close the client ensures resources are properly released when the test completes, regardless of whether the test passes or fails.
56-57: Simplified return statementThe return statement now only returns the publisher object without a cleanup function, improving readability and usability.
60-61: Cleaner test setupTest now simply calls
buildPublisherwithout having to manage cleanup, improving readability and reducing boilerplate.
115-116: Consistent application of cleanup patternSame clean pattern applied here as in the previous test.
167-168: Consistent cleanup pattern in concurrent testThe concurrent test also benefits from the simplified setup without explicit cleanup.
pkg/indexer/e2e_test.go (4)
24-28: Improved function signature with implicit cleanupThe function signature is cleaner now, returning only the necessary objects without the cleanup function. The cleanup is properly registered with
t.Cleanup(cancel)to ensure resources are released when the test completes.
32-33: Simplified anvil usageProperly uses the updated
anvil.StartAnvilfunction which now handles its own cleanup throught.Cleanup()internally.
49-50: Streamlined return valuesThe function now returns only the essential objects needed by the test, making it clearer what the function provides.
85-86: Cleaner test setupThe test now has a more straightforward setup without needing to manage cleanup explicitly, improving readability.
pkg/indexer/storer/identityUpdate_test.go (5)
24-28: Improved test helper signature with automatic cleanupThe function signature has been simplified by removing the cleanup function return. Context cancellation is now properly registered with
t.Cleanup(), ensuring resources are cleaned up automatically when the test completes.
30-31: Simplified anvil initializationCorrectly uses the updated
anvil.StartAnvilfunction which now handles its own cleanup internally, eliminating the need for explicit cleanup management.
48-49: Cleaner return statementThe function now returns only the storer and validation service without a cleanup function, simplifying the API and improving readability.
53-54: Streamlined test setupTest now simply calls
buildIdentityUpdateStorerwithout needing to capture or defer a cleanup function, reducing boilerplate code.
111-112: Consistent cleanup pattern applicationThe same cleanup pattern is consistently applied throughout the tests, ensuring all use the simplified approach.
pkg/api/message/subscribe_test.go (4)
28-31: Streamlined test setup functionThe function signature has been simplified by removing the cleanup function return. The code now properly relies on
t.Cleanupregistrations within theNewTestReplicationAPIClienthelper, improving code clarity.
89-90: Cleaner return statementThe function now returns only the necessary API client, database handle, and mocks without an explicit cleanup function, simplifying the API.
128-129: Simplified test implementationTest now calls
setupTestwithout capturing or deferring a cleanup function, making the test code more concise and easier to read.
148-149: Consistent cleanup pattern across all testsThe cleanup pattern is consistently applied throughout all test functions, ensuring a uniform approach to resource management.
Also applies to: 169-170, 190-191, 232-233, 253-254, 274-275
pkg/api/payer/publish_test.go (3)
69-97: Good refactoring to use t.Cleanup instead of returning a cleanup function.The function signature has been simplified by removing the return of a cleanup function, and instead using
t.Cleanup(cancel)to register the context cancellation. This is a cleaner approach that leverages Go's testing framework for automatic resource cleanup.
101-102: Clean update to the test call site.The call to
buildPayerServicehas been correctly updated to no longer capture or defer a cleanup function, matching the function's new signature.
176-180: Consistent application of the cleanup pattern.Both the
NewTestAPIServerandbuildPayerServicecalls have been updated to no longer capture or defer cleanup functions, maintaining consistency with the new approach of usingt.Cleanupinternally.pkg/testutils/api/api.go (4)
38-55: Excellent use of t.Cleanup for connection management.The function has been refactored to no longer return a cleanup function, instead using
t.Cleanupto register connection closure. This simplifies the function's interface and ensures cleanup happens automatically when the test completes.
76-92: Good refactoring to use t.Cleanup for connection management.Similar to
NewReplicationAPIClient, this function now usest.Cleanupto handle connection closure automatically, resulting in cleaner test code.
100-211: Good refactoring of NewTestAPIServer to use t.Cleanup.The function has been updated to register server shutdown and context cancellation with
t.Cleanupinstead of returning a cleanup function. This is a cleaner approach that automatically handles resource cleanup when tests complete.
213-227: Clean updates to test client creation functions.Both
NewTestReplicationAPIClientandNewTestMetadataAPIClienthave been correctly updated to no longer capture or defer cleanup functions, relying instead on the internalt.Cleanupcalls. This results in simpler, more maintainable test code.
neekolas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go 1.24 FTW
Modernize test cleanup by migrating test utility functions to use Go's built-in
|
Migrate test cleanup functions to use Go's built-in
t.Cleanupmechanism across test utilities and test filesThis change refactors test cleanup handling across multiple test files and utilities to use Go's built-in
t.Cleanupmechanism instead of explicitly returning and managing cleanup functions. Key changes include:StartAnvilin anvil.go to uset.Cleanupinstead of returning cleanup functionsNewReplicationAPIClient,NewMetadataAPIClient, andNewTestAPIServerto handle cleanup internallyTestCleanuptest in anvil_test.go as it's no longer relevant with the new cleanup approach📍Where to Start
Start with the core changes in anvil.go and api.go which define the new cleanup approach that propagates through all other test files.
Macroscope summarized ff36caf.
Summary by CodeRabbit
Refactor
Tests