Skip to content

fix: don't send heartbeat on some subscription resolver tests#1313

Merged
dkorittki merged 1 commit intotopic/streams-v1from
dominik/eng-8289-fix-flaky-engine-subscription-tests
Oct 9, 2025
Merged

fix: don't send heartbeat on some subscription resolver tests#1313
dkorittki merged 1 commit intotopic/streams-v1from
dominik/eng-8289-fix-flaky-engine-subscription-tests

Conversation

@dkorittki
Copy link
Copy Markdown
Contributor

@dkorittki dkorittki commented Oct 8, 2025

@coderabbitai summary

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.

I adjusted two tests which are failing during Githubs CI run:

--- FAIL: TestResolver_ResolveGraphQLSubscription (31.69s)
    --- FAIL: TestResolver_ResolveGraphQLSubscription/SubscriptionOnStart_ctx_updater_only_updates_the_right_subscription (0.01s)
        resolve_test.go:5604: 
            	Error Trace:	/home/runner/work/graphql-go-tools/graphql-go-tools/v2/pkg/engine/resolve/resolve_test.go:5604
            	Error:      	Should be true
            	Test:       	TestResolver_ResolveGraphQLSubscription/SubscriptionOnStart_ctx_updater_only_updates_the_right_subscription
    --- FAIL: TestResolver_ResolveGraphQLSubscription/SubscriptionOnStart_ctx_updater_on_multiple_subscriptions_with_same_trigger_works (0.01s)
        resolve_test.go:5674: 
            	Error Trace:	/home/runner/work/graphql-go-tools/graphql-go-tools/v2/pkg/engine/resolve/resolve_test.go:5674
            	Error:      	should not be here
            	Test:       	TestResolver_ResolveGraphQLSubscription/SubscriptionOnStart_ctx_updater_on_multiple_subscriptions_with_same_trigger_works
FAIL

These tests work locally, at least on my machine. I assumed it only manifests itself on slower CPU´s like it's the case on Github workers. After testing around with some time.Sleeps() here and there, I found that if messages in tests are not emitted fast enough, then heartbeat messages might find their way into the recorders message list. As these tests have nothing to do with heartbeat testing I disabled them here. I am not 100% sure this is the problem, but I am fairly certain and I think it makes sense anyway.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 8, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


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 merged commit 15d578b into topic/streams-v1 Oct 9, 2025
3 checks passed
@dkorittki dkorittki deleted the dominik/eng-8289-fix-flaky-engine-subscription-tests branch October 9, 2025 08:25
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.

3 participants