Skip to content

Comments

test: confirm socket mode receiver acknowledges processed events#2520

Merged
zimeg merged 3 commits intomainfrom
zimeg-test-socket-mode-ack
May 8, 2025
Merged

test: confirm socket mode receiver acknowledges processed events#2520
zimeg merged 3 commits intomainfrom
zimeg-test-socket-mode-ack

Conversation

@zimeg
Copy link
Member

@zimeg zimeg commented May 8, 2025

Summary

This PR adds test for Socket Mode event processing to ensure the received event is acknowledged as expected:

bolt-js/src/receivers/SocketModeReceiver.ts

Lines 226 to 247 in ee8df73

this.client.on('slack_event', async (args) => {
const { ack, body, retry_num, retry_reason } = args;
const event: ReceiverEvent = {
body,
ack,
retryNum: retry_num,
retryReason: retry_reason,
customProperties: customPropertiesExtractor(args),
};
try {
await this.app?.processEvent(event);
} catch (error) {
const shouldBeAcked = await this.processEventErrorHandler({
error: error as Error | CodedError,
logger: this.logger,
event,
});
if (shouldBeAcked) {
await ack();
}
}
});

Notes

Requirements

@zimeg zimeg requested a review from WilliamBergamin May 8, 2025 06:17
@zimeg zimeg self-assigned this May 8, 2025
@zimeg zimeg added the tests M-T: Testing work only label May 8, 2025
@codecov
Copy link

codecov bot commented May 8, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.24%. Comparing base (ee8df73) to head (e35e7cd).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2520      +/-   ##
==========================================
+ Coverage   92.97%   93.24%   +0.26%     
==========================================
  Files          36       36              
  Lines        7443     7443              
  Branches      648      652       +4     
==========================================
+ Hits         6920     6940      +20     
+ Misses        518      498      -20     
  Partials        5        5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zimeg zimeg mentioned this pull request May 8, 2025
Merged
2 tasks
WilliamBergamin
WilliamBergamin reviewed May 8, 2025
View reviewed changes
Copy link
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

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

Thanks for working on this these tests a very useful 💯

I'm just not sure this is the behavior we intend 🤔

WilliamBergamin
WilliamBergamin approved these changes May 8, 2025
View reviewed changes
Copy link
Contributor

@WilliamBergamin WilliamBergamin left a comment

Choose a reason for hiding this comment

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

This looks good 💯

Some tricky mocking but I think I understand it feel free to merge

zimeg
zimeg commented May 8, 2025
View reviewed changes
Copy link
Member Author

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

@WilliamBergamin Thanks for improving the test cases to match expected behaviors 🙏 ✨

I will rest better knowing these paths are now covered.

test/unit/receivers/SocketModeReceiver.spec.ts
Comment on lines +719 to +721
while (!defaultProcessEventErrorHandlerSpy.called) {
await delay(50);
}
Copy link
Member Author

@zimeg zimeg May 8, 2025

Choose a reason for hiding this comment

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

🧠 This is a super good case to cover too! We can now be more confident in retries on erroring cases I believe.

@zimeg zimeg added this to the 4.3.1 milestone May 8, 2025
@zimeg zimeg merged commit b0c9443 into main May 8, 2025
18 checks passed
@zimeg zimeg deleted the zimeg-test-socket-mode-ack branch May 8, 2025 21:41
@WilliamBergamin WilliamBergamin modified the milestones: 4.3.1, 4.4.0 May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WilliamBergamin WilliamBergamin WilliamBergamin approved these changes

Assignees

@zimeg zimeg

Labels

tests M-T: Testing work only

Projects

None yet

Milestone

4.4.0

Development

Successfully merging this pull request may close these issues.

2 participants

@zimeg @WilliamBergamin