Skip to content

Conversation

@sgoodrow
Copy link
Owner

@sgoodrow sgoodrow commented Jul 18, 2025

Establishes testing patterns for working with discord.js and implements a few example tests.

sgoodrow and others added 2 commits July 18, 2025 08:26
- Split monolithic discord-mocks.ts into focused, co-located files
- Created reusable mock helpers with proper Jest typing
- Implemented Pick<> types for Discord.js property inheritance
- Added type-safe conversion functions with controlled assertions
- Organized structure: utils/, mocks/, matchers/, helpers/
- Eliminated type casts and 'any' usage where possible
- Added comprehensive tests for applications feature

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Comment on lines 22 to 23
const { interaction, threadChannel } = setupApplicationTest();
const resolvedThreadChannel = await threadChannel;
Copy link
Owner Author

Choose a reason for hiding this comment

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

This is weird, just make setupApplicationTest async.

Comment on lines 31 to 33
expect(resolvedThreadChannel).toHaveSentMessage(
`Volunteer Application sent to **${interaction.user.username}** (<@${interaction.user.id}>)`
);
Copy link
Owner Author

Choose a reason for hiding this comment

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

This matcher should probably read more like 'toHaveReceivedMessage'

Comment on lines 38 to 41
const interaction = createMockButtonInteraction({ guild });

await expect(
requestApplication.execute(asButtonInteraction(interaction))
Copy link
Owner Author

Choose a reason for hiding this comment

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

Seems like the mock generator should already perform asButtonInteraction cast, no?


expect(buttonBuilder).toHaveCustomId("volunteer-application");
expect(buttonBuilder).toHaveLabel("Volunteer Application");
expect(buttonBuilder.data.style).toBe(1);
Copy link
Owner Author

Choose a reason for hiding this comment

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

What is 1? Maybe add a custom matcher for this

Comment on lines 79 to 89
const dmCall = interaction.user.send.mock.calls[0][0];
const content =
typeof dmCall === "string" ? dmCall : (dmCall as any).content;

expect(content).toContain("DO NOT REPLY TO THIS MESSAGE");
expect(content).toContain("How do I apply?");
expect(content).toContain(
"https://docs.google.com/forms/d/e/1FAIpQLSelYSgoouJCOIV9qoOQ1FdOXj8oGC2pfv7P47iUUd1hjOic-g/viewform"
);
expect(content).toContain("What happens to an application?");
expect(content).toContain("less than a week");
Copy link
Owner Author

Choose a reason for hiding this comment

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

This could be cleaner, both how we get the content and how we assert on it. Probably a helper for getting the content on the interaction mock, and a fixture for the assert.

Comment on lines 18 to 27
jest.mock("../../shared/action/instructions-ready-action", () => ({
InstructionsReadyAction: class {
createOrUpdateInstructions = mockCreateOrUpdateInstructions;
getChannel = mockGetChannel;
},
}));

jest.mock("../../shared/action/ready-action", () => ({
readyActionExecutor: jest.fn((action, options) => action.execute()),
}));
Copy link
Owner Author

Choose a reason for hiding this comment

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

Not crazy about this. Probably a cleaner way to do this. I expect we will need to do it a lot, also it looks like tight coupling. Maybe some IoC could fix this.

client = asClient(createMockClient());
// Create instance using constructor
action = new UpdateApplicationInfoAction(client);
jest.clearAllMocks();
Copy link
Owner Author

Choose a reason for hiding this comment

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

Just do this in a global before each if its needed. But also, is it?

let action: UpdateApplicationInfoAction;

beforeEach(() => {
client = asClient(createMockClient());
Copy link
Owner Author

Choose a reason for hiding this comment

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

Why not do the cast inside the mock generator?

it("should create instructions with volunteer roles embed and button", async () => {
await action.execute();

expect(mockCreateOrUpdateInstructions).toHaveBeenCalledWith(
Copy link
Owner Author

Choose a reason for hiding this comment

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

Not sure this is particularly useful as an assert. Maybe more we can do in the embed assert instead in order to check for other things like buttons.

describe("channel getter", () => {
it("should get applications channel with correct ID", () => {
// Access the protected property through reflection
const channel = (action as any).channel;
Copy link
Owner Author

Choose a reason for hiding this comment

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

ew, cast to any

) => readyActionExecutor(new UpdateApplicationInfoAction(client), options);

class UpdateApplicationInfoAction extends InstructionsReadyAction {
export class UpdateApplicationInfoAction extends InstructionsReadyAction {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Would be nice to not have to export this. Might mean tests need to interact at a higher level. That might also alleviate the need to mock that higher level tho.

createMockThreadChannel,
} from "./create-mock-thread-channel";

export type TestGuild = Pick<Guild, "id"> & {
Copy link
Owner Author

Choose a reason for hiding this comment

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

Standardize on these interface names. Do they start with Test or Mock?

sgoodrow and others added 14 commits July 18, 2025 11:45
- Eliminate asClient() and asButtonInteraction() casting
- Implement proper TypeScript structural typing for mocks
- Add integration testing via updateApplicationInfo
- Organize tests: integration tests + focused unit tests
- Standardize Mock* naming conventions
- Add shared mock utilities and better organization
- All 13 tests passing with cleaner, more maintainable code
- Added MSW setup for HTTP request mocking
- Created Discord API handlers and test examples
- Discovered MSW is not suitable for Discord.js testing because:
  * Discord.js abstracts away HTTP layer complexity
  * Heavy reliance on caching, rate limiting, WebSocket events
  * Would require reverse-engineering entire Discord API
  * Current mock-based approach is more appropriate
- Removed MSW experiment and dependencies
- Confirmed current testing strategy is the right approach
- Create setupDiscordMocks() and resetDiscordMocks() helpers
- Reduce Jest mocking boilerplate from 20+ lines to 2 lines
- Keep same test structure, just eliminate repetitive setup
- Easy to adopt in existing tests without architectural changes
- Sets foundation for future dependency injection refactor
Major improvements to Discord bot testing infrastructure:

## Testing Framework Enhancements
- **Beautiful BDD syntax**: `when("scenario", ({ it }) => { it("test", async ({ interaction, user }) => {...}) })`
- **Automatic setup**: No more manual imports or setup calls - objects provided as test parameters
- **Real integration testing**: Tests use `interactionCreateListener` to test actual Discord flow
- **Flexible config**: Optional config overrides per test block: `when("scenario", { config: {...} }, ({ it }) => {...})`

## Test Infrastructure
- Created `describe-discord.ts` with enhanced `when()` helper providing custom `it` function
- Created `discord-ui-expectations.ts` with reusable UI testing helpers
- Created `test-setup.ts` with `setupInteractionTest()` for consistent mock objects
- Enhanced `setup.ts` with automatic config mocking and comprehensive service mocks

## Developer Experience
- **Shared constants**: `features/applications/constants.ts` used by both production and test code
- **ESLint improvements**: Added `curly` rule requiring braces for all if statements
- **Better formatting**: 100 character line width for improved readability
- **Documentation**: Updated `CLAUDE.md` with successful testing patterns and philosophy

## API Examples
```typescript
// Simple test
when("handling volunteer applications", ({ it }) => {
  it("processes workflow correctly", async ({ interaction, threadChannel }) => {
    await requestApplication.execute(interaction);
    expect(threadChannel).toHaveReceivedMessage(...);
  });
});

// With custom config
when("special case", { config: { customValue: "test" } }, ({ it }) => {
  it("works with overrides", async ({ user, guild }) => {
    // Test logic here
  });
});
```

This establishes a beautiful, reusable testing framework that optimizes for both architectural soundness and developer joy.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Essential utility for mocking config values in tests, supports the new Discord testing framework.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Consistent formatting across all test helpers, mocks, and matchers following the new ESLint curly rule.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Consistent formatting across listeners, commands, actions, and shared utilities following the new ESLint curly rule.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Consistent formatting across applications, auctions, gatehouse-tags, threads, wakeup, and removed features following the new ESLint curly rule.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Consistent formatting across bank-hours, bank-inventory, bank-request-info, and jewelry-request-info features following the new ESLint curly rule.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Consistent formatting across bp, raid-bots, raid-schedule-info, and raider-enlistment features following the new ESLint curly rule.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Consistent formatting across dkp-records and invite-list features following the new ESLint curly rule.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Consistent formatting across services, database, and redis modules following the new ESLint curly rule.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Update VSCode settings and Jest configuration to align with new formatting and linting rules.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
…iscord testing library and custom matchers; wire global setup with reset calls; green tests
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