Add Vitest tests for Everything Server

[nielskaspers]

Summary

Adds comprehensive Vitest test coverage for the Everything Server, addressing #2925.

• 124 tests across tools, prompts, resources, and server
• 71% overall coverage (was 0%)
• All 16 tools, all 4 prompts tested

Coverage by Component

Component	Coverage	Tests
Tools	93%	61
Prompts	91%	18
Resources	65%	31
Server	63%	8
Registrations	100%	6

Tools (16/16 tested)

All tools at 100% except:

• get-roots-list (35%) - capability-gated branches
• gzip-file-as-resource (84%) - network error branches

Not Unit Tested

Transport files (stdio.ts, sse.ts, streamableHttp.ts) are executable entry points that start Express servers. These require integration tests rather than unit tests.

Test Files

__tests__/tools.test.ts        - 61 tests (all 16 tools)
__tests__/prompts.test.ts      - 18 tests (all 4 prompts)
__tests__/resources.test.ts    - 31 tests (templates, session, files, subscriptions)
__tests__/server.test.ts       - 8 tests (createServer factory)
__tests__/registrations.test.ts - 6 tests (index wiring files)
vitest.config.ts               - test configuration

Test Plan

• All 124 tests pass locally
• Coverage report generated with v8
• Mock server pattern matches existing test patterns in filesystem/memory servers

[olaservo]

Hi @nielskaspers hanks for adding test coverage to the Everything server! Going from 0% to meaningful coverage is a huge step forward, and the core handler behavior tests are solid. However, it looks like ~30 of the 124 tests could be improved or removed to keep the suite lean and meaningful:

---

1. Registration Boilerplate (16 tests)

Every tool and prompt has a "should register with correct name and config" test like this:

// tools.test.ts, prompts.test.ts — repeated ~15 times
it('should register with correct name and config', () => {
  expect(mockServer.registerTool).toHaveBeenCalledWith(
    'echo',
    expect.objectContaining({ title: 'Echo Tool', description: '...' }),
    expect.any(Function)
  );
});

These just assert that a string title/description was passed to registerTool. registrations.test.ts already verifies that all tools get registered by name — these per-tool variants add no behavioral coverage and will break on any description typo fix. Suggest removing all of them.

---

2. Redundant Role/Type Checks (3 tests)

In prompts.test.ts, three describe blocks each have a "should return message with user role" test:

// prompts.test.ts — appears in simple-prompt, args-prompt, and completable-prompt
it('should return message with user role', () => {
  const result = handler({ city: 'Boston' });
  expect(result.messages).toHaveLength(1);
  expect(result.messages[0].role).toBe('user');
  expect(result.messages[0].content.type).toBe('text');
});

The simple-prompt version is fully redundant — the preceding test already uses toEqual on the entire return value including role. The other two are partially redundant since prior tests in the same block already access messages[0] implicitly. Suggest consolidating these assertions into the existing behavioral tests.

---

3. "Should Not Throw" Tests With No Assertions (8 tests)

// resources.test.ts
it('should handle undefined sessionId', () => {
  expect(() => stopSimulatedResourceUpdates(undefined)).not.toThrow();
});

// server.test.ts
it('should cleanup without throwing errors', () => {
  expect(() => cleanup('test-session')).not.toThrow();
});

These verify a function doesn't crash but never check what it actually did. The 6 in resources.test.ts cover beginSimulatedResourceUpdates, stopSimulatedResourceUpdates, and registerFileResources. The 2 in server.test.ts cover cleanup with and without a session ID. None verify that state was actually cleaned up, intervals cleared, or resources registered. Suggest either adding meaningful assertions (e.g., verify intervals are cleared, state maps are empty) or removing these.

---

4. Constant Identity Tests (2 tests)

// resources.test.ts
it('should define text resource type', () => {
  expect(RESOURCE_TYPE_TEXT).toBe('Text');
});
it('should define blob resource type', () => {
  expect(RESOURCE_TYPE_BLOB).toBe('Blob');
});

Testing that a constant equals a string literal provides no safety net — if someone changes the constant, every consumer breaks regardless. Suggest removing.

---

5. expect(true).toBe(true)

// resources.test.ts
it('should register file resources from docs directory', () => {
  registerFileResources(mockServer);
  expect(true).toBe(true);
});

This always passes and tests nothing. Suggest removing or replacing with an actual assertion (e.g., check registerResource was called with expected filenames).

---

6. Weak Assertions (2 tests)

// server.test.ts — claims to test capability but doesn't
it('should have tools capability enabled', () => {
  const { server } = createServer();
  expect(server).toBeDefined();  // doesn't check capabilities
});

// registrations.test.ts
it('should register resource templates', () => {
  registerResources(mockServer);
  expect(mockServer.registerResource).toHaveBeenCalled();  // no count, no names
});

Suggest either strengthening the assertions or removing the tests.

---

Summary

~31 of the 124 tests (16 registration boilerplate + 3 redundant role checks + 8 "should not throw" + 2 constant identity + 1 expect(true) + 1 weak "capability" test) are padding that could be removed or consolidated. The remaining ~93 are genuinely useful. Trimming the low-value tests would make the suite more maintainable and the coverage numbers more honest.

(Note, Claude was used to help generate this assessment and verify this feedback.)

[nielskaspers]

Hi @olaservo! Thank you for the thorough review — you're absolutely right that those tests were adding noise rather than meaningful coverage. I really appreciate you taking the time to identify them so specifically.

I've addressed all your feedback:

Removed (29 tests):

• ✅ Registration boilerplate (16) — redundant with registrations.test.ts
• ✅ Redundant role/type checks (3) — consolidated assertions into existing behavioral tests
• ✅ "Should not throw" tests (6) — merged into a single lifecycle test
• ✅ Constant identity tests (2) — agreed, these provided no safety net
• ✅ expect(true).toBe(true) (1) — replaced with actual assertion that registerResource was called
• ✅ Weak capability test (1) — removed, the oninitialized handler check already exists

Strengthened:

• Resource templates test now verifies specific resource names (Dynamic Text Resource, Dynamic Blob Resource)
• File resources test now asserts registerResource was called

Test count went from 124 → 95, and all tests pass. Coverage remains at ~71%.

Let me know if there's anything else you'd like me to adjust!

[olaservo]

Thanks @nielskaspers !