Skip to content

fix: implement missing subscription updater methods#1310

Merged
dkorittki merged 2 commits intotopic/streams-v1from
dominik/eng-8279-implement-missing-interface-methods-on-engine-tests
Oct 6, 2025
Merged

fix: implement missing subscription updater methods#1310
dkorittki merged 2 commits intotopic/streams-v1from
dominik/eng-8279-implement-missing-interface-methods-on-engine-tests

Conversation

@dkorittki
Copy link
Copy Markdown
Contributor

@dkorittki dkorittki commented Oct 6, 2025

Summary by CodeRabbit

  • Tests
    • Enhanced subscription-related test coverage for the GraphQL data source, aligning test utilities with current interfaces to improve reliability and maintainability.
    • Validated subscribe/unsubscribe and update flows under various scenarios to reduce brittleness and increase confidence in behavior.
    • Performed minor cleanup in test code to improve readability without altering functionality.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.

Implements missing methods to satisfy the resolve.SubscriptionUpdater interface, so tests can build. Since these methods are not used by the tests, they are empty. There tests in place already in resolve_test.go, which test wether the updater can manage subscriptions individually, so I did not add any.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 6, 2025

Walkthrough

Adds three method implementations to testSubscriptionUpdater to satisfy a test interface and removes an empty line in FailingSubscriptionClient.Unsubscribe, all within the GraphQL datasource test file.

Changes

Cohort / File(s) Summary
GraphQL datasource tests
v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go
Implemented testSubscriptionUpdater methods: CloseSubscription(kind resolve.SubscriptionCloseKind, id resolve.SubscriptionIdentifier), Subscriptions() map[context.Context]resolve.SubscriptionIdentifier, UpdateSubscription(id resolve.SubscriptionIdentifier, data []byte). Removed a blank line in FailingSubscriptionClient.Unsubscribe.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the main change of implementing missing subscription updater methods to satisfy the interface in tests and follows conventional commit style. It is specific, clear, and directly related to the code modifications without including extraneous details. This makes it easy for teammates to understand the primary intent at a glance.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d6b668 and 9d03e77.

📒 Files selected for processing (1)
  • v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go (1 hunks)
🔇 Additional comments (1)
v2/pkg/engine/datasource/graphql_datasource/graphql_datasource_test.go (1)

8333-8344: Stub methods look good.

The new implementations cleanly satisfy the updated resolve.SubscriptionUpdater contract without impacting the existing test behavior. Nicely handled.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@dkorittki dkorittki changed the title fix: test subscription updaters ability to manage individual subscribers fix: implement missing subscription updater methods Oct 6, 2025
@dkorittki dkorittki marked this pull request as ready for review October 6, 2025 12:23
@dkorittki dkorittki requested a review from alepane21 October 6, 2025 12:25
@alepane21
Copy link
Copy Markdown
Contributor

@coderabbitai full review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 6, 2025

✅ Actions performed

Full review triggered.

@dkorittki dkorittki merged commit 2391c87 into topic/streams-v1 Oct 6, 2025
4 checks passed
@dkorittki dkorittki deleted the dominik/eng-8279-implement-missing-interface-methods-on-engine-tests branch October 6, 2025 12:31
@coderabbitai coderabbitai Bot mentioned this pull request Mar 23, 2026
3 tasks
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