feat(tests): clean connectors for every tests #409
Conversation
WalkthroughThis pull request updates the project’s configuration and end-to-end test suites. The Changes
Sequence Diagram(s)sequenceDiagram
participant TR as Test Runner
participant BE as BeforeEach Hook
participant TC as Test Case
participant AE as AfterEach Hook
participant CT as Connector
TR->>BE: Execute setup
BE->>CT: Install connector (or prepare UUID)
TR->>TC: Run test scenario
TC->>TR: Perform assertions
TR->>AE: Execute teardown
AE->>CT: Uninstall connector
Assessment against linked issues
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
6d7db33 to
425598d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #409 +/- ##
=======================================
Coverage 59.63% 59.63%
=======================================
Files 538 538
Lines 26734 26734
=======================================
Hits 15942 15942
Misses 9953 9953
Partials 839 839 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/e2e/api_pools_test.go (1)
69-70: Simplification of expectation.The timeout specification was removed from the
Eventuallymatcher. This is generally acceptable if the default timeout is sufficient for the test case to complete.However, if this test is occasionally flaky or takes longer than expected, you might want to consider explicitly setting an appropriate timeout.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.gitignore(1 hunks)test/e2e/api_accounts_test.go(3 hunks)test/e2e/api_bank_accounts_test.go(8 hunks)test/e2e/api_connectors_test.go(7 hunks)test/e2e/api_payment_initiations_test.go(11 hunks)test/e2e/api_payments_test.go(4 hunks)test/e2e/api_pools_test.go(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
test/e2e/api_payment_initiations_test.go (7)
pkg/testserver/matchers.go (4)
Event(142-147)WithPayloadSubset(92-97)HaveTaskStatus(184-189)WithError(212-216)pkg/events/event.go (4)
EventTypeSavedPaymentInitiation(16-16)EventTypeSavedPaymentInitiationAdjustment(17-17)EventTypeSavedPayments(11-11)EventTypeSavedPaymentInitiationRelatedPayment(18-18)internal/models/payment_initiation_adjusments_status.go (2)
PAYMENT_INITIATION_ADJUSTMENT_STATUS_WAITING_FOR_VALIDATION(14-14)PAYMENT_INITIATION_ADJUSTMENT_STATUS_PROCESSED(16-16)pkg/testserver/helpers.go (1)
TaskPoller(41-43)internal/models/task_id.go (1)
TaskID(12-15)internal/models/tasks.go (2)
TASK_STATUS_SUCCEEDED(15-15)TASK_STATUS_FAILED(16-16)internal/connectors/engine/workflow/reverse.go (1)
ErrPaymentInitiationNotProcessed(17-17)
test/e2e/api_connectors_test.go (6)
pkg/testserver/server.go (1)
Server(61-71)internal/models/task_id.go (2)
TaskIDFromString(37-49)TaskID(12-15)pkg/testserver/helpers.go (1)
TaskPoller(41-43)internal/models/plugin.go (1)
DefaultConnectorClientTimeout(9-9)pkg/testserver/matchers.go (1)
HaveTaskStatus(184-189)internal/models/tasks.go (1)
TASK_STATUS_SUCCEEDED(15-15)
test/e2e/api_pools_test.go (2)
pkg/testserver/helpers.go (1)
Subscribe(30-39)pkg/testserver/matchers.go (2)
Event(142-147)WithPayloadSubset(92-97)
test/e2e/api_accounts_test.go (5)
internal/connectors/plugins/public/dummypay/plugin.go (1)
New(27-38)pkg/testserver/helpers.go (1)
Subscribe(30-39)pkg/testserver/matchers.go (2)
Event(142-147)WithCallback(45-50)pkg/events/event.go (2)
EventTypeSavedAccounts(12-12)EventTypeSavedBalances(13-13)internal/events/balance.go (1)
BalanceMessagePayload(12-20)
🔇 Additional comments (26)
.gitignore (1)
12-13: Well-organized repository structure updates.The additions to .gitignore look appropriate: re-adding the connector-template directory and adding coverage.txt to avoid tracking test coverage reports in version control.
test/e2e/api_payments_test.go (4)
51-53: Proper test lifecycle standardization.Replacing
JustBeforeEachwithBeforeEachensures setup happens at the right moment in the Ginkgo test lifecycle. This change promotes better test isolation.
61-63: Good cleanup practice added.Adding the
AfterEachblock to uninstall the connector after each test ensures proper cleanup of resources, preventing state leakage between tests.
113-121: Proper test lifecycle standardization.Similar to the v3 test case, this standardizes the setup timing by using
BeforeEachinstead ofJustBeforeEach.
123-125: Good cleanup practice added.Adding the
AfterEachblock to uninstall the connector after the v2 tests maintains consistency with the v3 tests and ensures proper resource cleanup.test/e2e/api_connectors_test.go (3)
48-50: Improved test isolation for UUID generation.Standardizing the use of
BeforeEachfor UUID generation ensures each test gets a fresh UUID, improving test isolation and predictability.
140-142: Consistent test setup timing.Standardizing the use of
BeforeEachacross all test cases ensures consistent behavior in the test lifecycle. This makes tests more reliable and maintainable.Also applies to: 217-219, 253-255, 290-306, 331-340
522-539: Well-structured connector cleanup function.The new
uninstallConnectorfunction properly encapsulates the logic for uninstalling a connector and verifying the task completion. This centralized approach ensures consistent cleanup across all tests.The function:
- Calls the SDK's UninstallConnector method
- Verifies the task ID is valid and contains "uninstall"
- Sets up a task poller to monitor the uninstallation
- Blocks until the workflow completes
- Verifies that the task status is SUCCEEDED
This implementation aligns perfectly with the PR objective of properly cleaning connectors for every test.
test/e2e/api_pools_test.go (2)
49-53: Standardized test setup timing.Consistently using
BeforeEachinstead ofJustBeforeEachacross all test cases improves test organization and ensures setup happens at the same phase in the test lifecycle.Also applies to: 120-124, 196-212, 262-278
55-57: Proper test resource cleanup.Adding
AfterEachblocks to uninstall connectors after each test ensures proper cleanup of resources, preventing state leakage between tests and potential resource exhaustion during test runs.Also applies to: 126-128, 214-216, 280-282
test/e2e/api_payment_initiations_test.go (7)
56-87: Improved test setup and teardown via BeforeEach & AfterEachThe change from
JustBeforeEachtoBeforeEachensures that connector installation and subscription happen at the right stage in the test lifecycle. Adding theAfterEachblock for uninstalling the connector ensures proper cleanup after each test, preventing potential test pollution.
80-86: Simplified Eventually assertionsRemoved the unnecessary
.WithTimeout(2 * time.Second)from theseEventuallyassertions, making the code more concise while relying on default timeout settings.
89-91: Added connector cleanup in AfterEach blockThis addition ensures that connectors are properly uninstalled after each test, which improves test isolation and prevents potential interference between tests.
118-124: Simplified Eventually assertionsRemoved the
.WithTimeout(2 * time.Second)from these assertions, making the code more concise while maintaining the same behavior through default timeout settings.
126-126: Adjusted taskPoller timeoutThe timeout for the task polling operation has been adjusted to 2 seconds, providing a more appropriate constraint for expected response times.
205-229: Improved test setup for payout initiation testsSimilar to the previous test block, changing to
BeforeEachimproves the test lifecycle management for the payout initiation tests.
231-233: Added connector cleanup for payout testsThis
AfterEachblock ensures proper cleanup of connectors after payout initiation tests, consistent with the pattern established in other test blocks.test/e2e/api_bank_accounts_test.go (5)
65-69: Updated test setup timing with BeforeEachChanged from
JustBeforeEachtoBeforeEachto ensure the setup code executes at the appropriate time in the test lifecycle, improving test predictability.
107-117: Restructured connector setup for bank account forwarding testsMoved setup code to
BeforeEachto ensure proper initialization before 8000 each test runs, improving test isolation and reliability.
119-121: Added connector cleanup for bank account forwarding testsThis
AfterEachblock ensures connectors are properly uninstalled after each test, preventing potential interference between tests and resource leaks.
184-193: Updated setup for v2 bank account forwarding testsChanged setup timing from
JustBeforeEachtoBeforeEachfor consistency with other test blocks and improved test lifecycle management.
195-197: Added connector cleanup for v2 bank account forwarding testsEnsures proper cleanup of connectors after each v2 bank account forwarding test, consistent with the cleanup pattern established in other test blocks.
test/e2e/api_accounts_test.go (4)
49-52: Improved variable declarationsAdded explicit variable declarations for
connectorIDanderrat the test block level, making them accessible throughout the test scope while improving code organization.
54-61: Added proper setup and teardown for account creation testsMoved connector installation to
BeforeEachand addedAfterEachto uninstall the connector after each test. This ensures proper test isolation and prevents resource leaks.
110-113: Enhanced variable declarations for account balances testsAdded explicit variable declarations for
connectorIDanderrat the test block level, consistent with the pattern in other test blocks, improving code organization.
126-128: Added connector cleanup for account balances testsThis
AfterEachblock ensures connectors are properly uninstalled after each account balances test, improving test isolation and resource management.
| AfterEach(func() { | ||
| uninstallConnector(ctx, app.GetValue(), connectorID) | ||
| }) |
There was a problem hiding this comment.
In the future it might not be a bad idea to implement a t.CleanupTest() type function that allows us to setup the AfterEach tear down functions in the constructors.
There was a problem hiding this comment.
Just in case you don't know, you can use DeferCleanup
Fixes PMNT-86