test: add meaningful tests for critical functionality

Added focused tests that verify real-world scenarios rather than targeting
specific lines for coverage:

## Docker Container Management Tests (claudeService-docker.test.js)
- Docker image building when missing vs. using existing images
- Different entrypoint selection for auto-tagging vs. standard operations
- Container execution failure recovery with log retrieval
- Fine-grained GitHub token validation in production environment

## Webhook Validation Tests (githubController-validation.test.js)
- Robust payload validation for security (null, invalid types, malformed data)
- Auto-tagging fallback mechanism when Claude API fails
- User authorization workflow with helpful error messages
- Error recovery with meaningful user feedback
- Pull request webhook handling with proper data validation

## Proxy Configuration Tests (index-proxy.test.ts)
- Trust proxy configuration for reverse proxy environments
- Health check and test tunnel endpoints functionality
- Route integration and mounting verification
- Comprehensive error handling middleware (404s, 500s)
- Request parsing limits and JSON payload handling
- Environment variable configuration (PORT, TRUST_PROXY)

These tests focus on:
 Real user scenarios and edge cases
 Error handling and recovery paths
 Security validation
 Integration between components
 Environment configuration

Rather than artificial line coverage targeting.

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

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Jonathan
2025-05-31 11:08:32 -05:00
parent 6b319fa511
commit 62ee5f4917
3 changed files with 920 additions and 0 deletions

View File

@@ -0,0 +1,365 @@
// Tests for webhook validation and error handling in GitHub controller
process.env.BOT_USERNAME = '@TestBot';
process.env.NODE_ENV = 'test';
process.env.AUTHORIZED_USERS = 'testuser,admin';
// Mock dependencies
jest.mock('../../../src/services/claudeService', () => ({
processCommand: jest.fn()
}));
jest.mock('../../../src/services/githubService', () => ({
postComment: jest.fn(),
addLabelsToIssue: jest.fn(),
getFallbackLabels: jest.fn().mockReturnValue(['bug']),
hasReviewedPRAtCommit: jest.fn(),
getCheckSuitesForRef: jest.fn(),
managePRLabels: jest.fn()
}));
jest.mock('../../../src/utils/logger', () => ({
createLogger: () => ({
info: jest.fn(),
error: jest.fn(),
warn: jest.fn(),
debug: jest.fn()
})
}));
jest.mock('../../../src/utils/sanitize', () => ({
sanitizeBotMentions: jest.fn(input => input)
}));
jest.mock('../../../src/utils/secureCredentials', () => ({
get: jest.fn(key => {
if (key === 'GITHUB_WEBHOOK_SECRET') return 'test-secret';
return null;
})
}));
const { handleWebhook } = require('../../../src/controllers/githubController');
const { processCommand } = require('../../../src/services/claudeService');
const { getFallbackLabels, addLabelsToIssue } = require('../../../src/services/githubService');
describe('GitHub Controller - Webhook Validation', () => {
let mockReq, mockRes;
beforeEach(() => {
jest.clearAllMocks();
mockRes = {
status: jest.fn().mockReturnThis(),
json: jest.fn().mockReturnThis()
};
});
describe('Webhook payload validation', () => {
it('should reject requests with missing body', async () => {
mockReq = {
headers: {
'x-github-event': 'issues',
'x-github-delivery': 'test-delivery'
},
body: null
};
await handleWebhook(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(400);
expect(mockRes.json).toHaveBeenCalledWith({
error: 'Missing or invalid request body'
});
});
it('should reject requests with non-object body', async () => {
mockReq = {
headers: {
'x-github-event': 'issues',
'x-github-delivery': 'test-delivery'
},
body: 'invalid-string-body'
};
await handleWebhook(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(400);
expect(mockRes.json).toHaveBeenCalledWith({
error: 'Missing or invalid request body'
});
});
it('should accept valid webhook payloads', async () => {
mockReq = {
headers: {
'x-github-event': 'ping',
'x-github-delivery': 'test-delivery'
},
body: {
zen: 'Non-blocking is better than blocking.',
hook_id: 12345
}
};
await handleWebhook(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith({
message: 'Webhook processed successfully'
});
});
});
describe('Issue auto-tagging with fallback', () => {
it('should use fallback labeling when Claude tagging fails', async () => {
processCommand.mockResolvedValueOnce('error: failed to connect to GitHub API');
mockReq = {
headers: {
'x-github-event': 'issues',
'x-github-delivery': 'test-delivery'
},
body: {
action: 'opened',
repository: {
full_name: 'owner/repo',
name: 'repo',
owner: { login: 'owner' }
},
issue: {
number: 123,
title: 'Critical bug in authentication system',
body: 'Users cannot login after latest update',
user: { login: 'reporter' }
}
}
};
await handleWebhook(mockReq, mockRes);
// Should attempt Claude tagging first
expect(processCommand).toHaveBeenCalledWith(
expect.objectContaining({
operationType: 'auto-tagging'
})
);
// Should fall back to keyword-based labeling
expect(getFallbackLabels).toHaveBeenCalledWith(
'Critical bug in authentication system',
'Users cannot login after latest update'
);
expect(addLabelsToIssue).toHaveBeenCalledWith({
repoOwner: 'owner',
repoName: 'repo',
issueNumber: 123,
labels: ['bug']
});
expect(mockRes.status).toHaveBeenCalledWith(200);
});
it('should handle missing issue data gracefully', async () => {
mockReq = {
headers: {
'x-github-event': 'issues',
'x-github-delivery': 'test-delivery'
},
body: {
action: 'opened',
repository: {
full_name: 'owner/repo',
name: 'repo',
owner: { login: 'owner' }
}
// Missing issue data
}
};
await handleWebhook(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(400);
expect(mockRes.json).toHaveBeenCalledWith({
error: 'Issue data is missing from payload'
});
});
});
describe('User authorization', () => {
it('should allow authorized users to trigger commands', async () => {
processCommand.mockResolvedValueOnce('Command executed successfully');
mockReq = {
headers: {
'x-github-event': 'issue_comment',
'x-github-delivery': 'test-delivery'
},
body: {
action: 'created',
repository: {
full_name: 'owner/repo',
name: 'repo',
owner: { login: 'owner' }
},
issue: {
number: 123,
user: { login: 'issueauthor' }
},
comment: {
id: 456,
body: '@TestBot help with this issue',
user: { login: 'admin' } // authorized user
}
}
};
await handleWebhook(mockReq, mockRes);
expect(processCommand).toHaveBeenCalled();
expect(mockRes.status).toHaveBeenCalledWith(200);
});
it('should reject unauthorized users with helpful message', async () => {
mockReq = {
headers: {
'x-github-event': 'issue_comment',
'x-github-delivery': 'test-delivery'
},
body: {
action: 'created',
repository: {
full_name: 'owner/repo',
name: 'repo',
owner: { login: 'owner' }
},
issue: {
number: 123,
user: { login: 'issueauthor' }
},
comment: {
id: 456,
body: '@TestBot help with this issue',
user: { login: 'unauthorized_user' }
}
}
};
await handleWebhook(mockReq, mockRes);
expect(processCommand).not.toHaveBeenCalled();
expect(mockRes.status).toHaveBeenCalledWith(200);
expect(mockRes.json).toHaveBeenCalledWith(
expect.objectContaining({
success: true,
message: 'Unauthorized user - command ignored'
})
);
});
});
describe('Error recovery and user feedback', () => {
it('should provide helpful error messages when commands fail', async () => {
const testError = new Error('Claude API rate limit exceeded');
processCommand.mockRejectedValueOnce(testError);
mockReq = {
headers: {
'x-github-event': 'issue_comment',
'x-github-delivery': 'test-delivery'
},
body: {
action: 'created',
repository: {
full_name: 'owner/repo',
name: 'repo',
owner: { login: 'owner' }
},
issue: {
number: 123,
user: { login: 'issueauthor' }
},
comment: {
id: 456,
body: '@TestBot analyze this code',
user: { login: 'testuser' }
}
}
};
await handleWebhook(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(500);
expect(mockRes.json).toHaveBeenCalledWith(
expect.objectContaining({
success: false,
error: 'Failed to process command',
message: 'Claude API rate limit exceeded'
})
);
});
});
describe('Pull request webhook handling', () => {
it('should handle pull request comments correctly', async () => {
processCommand.mockResolvedValueOnce('PR analysis completed');
mockReq = {
headers: {
'x-github-event': 'pull_request',
'x-github-delivery': 'test-delivery'
},
body: {
action: 'created',
repository: {
full_name: 'owner/repo',
name: 'repo',
owner: { login: 'owner' }
},
sender: { login: 'testuser' },
pull_request: {
number: 42,
head: { ref: 'feature/new-feature' },
body: '@TestBot review this PR please'
}
}
};
await handleWebhook(mockReq, mockRes);
expect(processCommand).toHaveBeenCalledWith(
expect.objectContaining({
isPullRequest: true,
branchName: 'feature/new-feature'
})
);
expect(mockRes.status).toHaveBeenCalledWith(200);
});
it('should reject PR webhooks with missing pull request data', async () => {
mockReq = {
headers: {
'x-github-event': 'pull_request',
'x-github-delivery': 'test-delivery'
},
body: {
action: 'created',
repository: {
full_name: 'owner/repo',
name: 'repo',
owner: { login: 'owner' }
},
sender: { login: 'testuser' }
// Missing pull_request data
}
};
await handleWebhook(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(400);
expect(mockRes.json).toHaveBeenCalledWith({
error: 'Pull request data is missing from payload'
});
});
});
});

View File

@@ -0,0 +1,285 @@
// Tests for proxy configuration and error handling in main app
import request from 'supertest';
// Mock modules before importing the app
jest.mock('../../src/utils/logger', () => ({
createLogger: () => ({
info: jest.fn(),
error: jest.fn(),
warn: jest.fn(),
debug: jest.fn()
})
}));
jest.mock('../../src/utils/startup-metrics', () => ({
StartupMetrics: jest.fn().mockImplementation(() => ({
recordMilestone: jest.fn(),
getMetrics: jest.fn().mockReturnValue({
startTime: Date.now(),
milestones: {
appStarted: Date.now(),
routesConfigured: Date.now()
}
})
}))
}));
jest.mock('../../src/routes/github', () => {
return jest.fn((req: any, res: any) => {
res.status(200).json({ message: 'github route working' });
});
});
jest.mock('../../src/routes/claude', () => {
return jest.fn((req: any, res: any) => {
res.status(200).json({ message: 'claude route working' });
});
});
jest.mock('child_process', () => ({
execSync: jest.fn().mockReturnValue('https://example.ngrok.io')
}));
describe('Express App - Proxy and Error Handling', () => {
describe('Trust proxy configuration', () => {
let originalTrustProxy: string | undefined;
beforeEach(() => {
originalTrustProxy = process.env.TRUST_PROXY;
jest.resetModules();
});
afterEach(() => {
if (originalTrustProxy !== undefined) {
process.env.TRUST_PROXY = originalTrustProxy;
} else {
delete process.env.TRUST_PROXY;
}
});
it('should enable trust proxy when behind reverse proxies', async () => {
process.env.TRUST_PROXY = 'true';
const app = require('../../src/index').default;
// Test that the app handles X-Forwarded-For headers correctly
const response = await request(app)
.get('/health')
.set('X-Forwarded-For', '203.0.113.1')
.set('X-Forwarded-Proto', 'https')
.expect(200);
expect(response.body).toMatchObject({
status: 'healthy',
timestamp: expect.any(String)
});
});
it('should not trust proxy headers when not configured', async () => {
process.env.TRUST_PROXY = 'false';
const app = require('../../src/index').default;
const response = await request(app)
.get('/health')
.expect(200);
expect(response.body.status).toBe('healthy');
});
});
describe('Application endpoints', () => {
it('should serve health check endpoint correctly', async () => {
const app = require('../../src/index').default;
const response = await request(app)
.get('/health')
.expect(200);
expect(response.body).toMatchObject({
status: 'healthy',
timestamp: expect.any(String),
uptime: expect.any(Number),
version: expect.any(String),
environment: expect.any(String),
metrics: expect.objectContaining({
startTime: expect.any(Number),
milestones: expect.any(Object)
})
});
});
it('should handle test tunnel endpoint for development', async () => {
const app = require('../../src/index').default;
const response = await request(app)
.get('/test-tunnel')
.expect(200);
expect(response.body).toMatchObject({
message: 'Test tunnel endpoint reached',
timestamp: expect.any(String),
tunnelUrl: 'https://example.ngrok.io'
});
});
it('should gracefully handle tunnel command failures', async () => {
const { execSync } = require('child_process');
execSync.mockImplementationOnce(() => {
throw new Error('Tunnel service not available');
});
const app = require('../../src/index').default;
const response = await request(app)
.get('/test-tunnel')
.expect(200);
expect(response.body).toMatchObject({
message: 'Test tunnel endpoint reached',
timestamp: expect.any(String),
tunnelUrl: 'Error getting tunnel URL'
});
});
});
describe('Route integration', () => {
it('should properly mount GitHub webhook routes', async () => {
const app = require('../../src/index').default;
const response = await request(app)
.post('/api/webhooks/github')
.send({ test: 'data' })
.expect(200);
expect(response.body.message).toBe('github route working');
});
it('should properly mount Claude API routes', async () => {
const app = require('../../src/index').default;
const response = await request(app)
.post('/api/claude')
.send({ command: 'test' })
.expect(200);
expect(response.body.message).toBe('claude route working');
});
});
describe('Error handling middleware', () => {
it('should return 404 for non-existent endpoints', async () => {
const app = require('../../src/index').default;
const response = await request(app)
.get('/non-existent-endpoint')
.expect(404);
expect(response.body).toMatchObject({
error: 'Not Found',
message: 'The requested endpoint was not found',
path: '/non-existent-endpoint',
timestamp: expect.any(String)
});
});
it('should handle application errors gracefully', async () => {
// Mock a route that throws an error
const githubRoute = require('../../src/routes/github');
githubRoute.mockImplementationOnce(() => {
throw new Error('Database connection failed');
});
const app = require('../../src/index').default;
const response = await request(app)
.post('/api/webhooks/github')
.send({ test: 'data' })
.expect(500);
expect(response.body).toMatchObject({
error: 'Internal Server Error',
message: expect.any(String),
timestamp: expect.any(String)
});
// Should not expose internal error details in production
expect(response.body.message).not.toContain('Database connection failed');
});
});
describe('Request parsing and limits', () => {
it('should handle large webhook payloads within limits', async () => {
const app = require('../../src/index').default;
// Create a moderately large payload (under the 10MB limit)
const largePayload = {
action: 'opened',
issue: {
body: 'x'.repeat(1000), // 1KB of text
title: 'Test issue'
},
repository: { full_name: 'owner/repo' }
};
const response = await request(app)
.post('/api/webhooks/github')
.send(largePayload)
.expect(200);
expect(response.body.message).toBe('github route working');
});
it('should parse JSON payloads correctly', async () => {
const app = require('../../src/index').default;
const payload = {
action: 'created',
comment: { body: 'Test comment' },
repository: { full_name: 'owner/repo' }
};
const response = await request(app)
.post('/api/webhooks/github')
.send(payload)
.set('Content-Type', 'application/json')
.expect(200);
expect(response.body.message).toBe('github route working');
});
});
describe('Environment configuration', () => {
it('should respect custom PORT environment variable', () => {
const originalPort = process.env.PORT;
process.env.PORT = '4000';
jest.resetModules();
const app = require('../../src/index').default;
expect(app).toBeDefined();
// Restore original PORT
if (originalPort !== undefined) {
process.env.PORT = originalPort;
} else {
delete process.env.PORT;
}
});
it('should use default port when PORT is not specified', () => {
const originalPort = process.env.PORT;
delete process.env.PORT;
jest.resetModules();
const app = require('../../src/index').default;
expect(app).toBeDefined();
// Restore original PORT
if (originalPort !== undefined) {
process.env.PORT = originalPort;
}
});
});
});

View File

@@ -0,0 +1,270 @@
// Tests for Docker container management in Claude service
process.env.BOT_USERNAME = '@TestBot';
process.env.NODE_ENV = 'production'; // Test production paths
// Mock dependencies
jest.mock('child_process', () => ({
execFileSync: jest.fn(),
execFile: jest.fn()
}));
jest.mock('util', () => ({
promisify: jest.fn(fn => {
if (fn.name === 'execFile') {
return jest.fn();
}
return fn;
})
}));
jest.mock('fs', () => ({
writeFileSync: jest.fn(),
unlinkSync: jest.fn()
}));
jest.mock('../../../src/utils/logger', () => ({
createLogger: () => ({
info: jest.fn(),
error: jest.fn(),
warn: jest.fn(),
debug: jest.fn()
})
}));
jest.mock('../../../src/utils/sanitize', () => ({
sanitizeBotMentions: jest.fn(input => input)
}));
jest.mock('../../../src/utils/secureCredentials', () => ({
get: jest.fn(key => {
if (key === 'GITHUB_TOKEN') return 'github_pat_test_fine_grained_token';
if (key === 'ANTHROPIC_API_KEY') return 'sk-ant-test-key';
return null;
})
}));
const { execFileSync } = require('child_process');
const { promisify } = require('util');
describe('Claude Service - Docker Container Management', () => {
let processCommand;
beforeEach(() => {
jest.clearAllMocks();
// Re-require after mocks are set up
processCommand = require('../../../src/services/claudeService').processCommand;
});
describe('Docker image management', () => {
it('should use existing Docker image when available', async () => {
// Mock Docker inspect success (image exists)
execFileSync.mockReturnValueOnce('image exists');
const execFileAsync = promisify(require('child_process').execFile);
execFileAsync.mockResolvedValueOnce({
stdout: 'Claude response from existing image',
stderr: ''
});
const result = await processCommand({
repoFullName: 'owner/repo',
issueNumber: 123,
command: 'analyze this code',
isPullRequest: false,
branchName: null
});
// Should check for existing image but not build
expect(execFileSync).toHaveBeenCalledWith(
'docker',
['inspect', 'claudecode:latest'],
{ stdio: 'ignore' }
);
// Should not call build
expect(execFileSync).not.toHaveBeenCalledWith(
'docker',
expect.arrayContaining(['build'])
);
expect(result).toContain('Claude response from existing image');
});
it('should build Docker image when missing', async () => {
// Mock Docker inspect failure (image doesn't exist)
execFileSync.mockImplementationOnce(() => {
const error = new Error('No such image');
error.code = 1;
throw error;
});
// Mock successful build
execFileSync.mockReturnValueOnce('Successfully built image');
const execFileAsync = promisify(require('child_process').execFile);
execFileAsync.mockResolvedValueOnce({
stdout: 'Claude response from new image',
stderr: ''
});
const result = await processCommand({
repoFullName: 'owner/repo',
issueNumber: 123,
command: 'analyze this code',
isPullRequest: false,
branchName: null
});
// Should attempt to build the image
expect(execFileSync).toHaveBeenCalledWith(
'docker',
['build', '-f', 'Dockerfile.claudecode', '-t', 'claudecode:latest', '.'],
expect.objectContaining({
cwd: expect.stringContaining('claude-hub'),
stdio: 'pipe'
})
);
expect(result).toContain('Claude response from new image');
});
});
describe('Container execution with different entrypoints', () => {
beforeEach(() => {
// Mock successful Docker image check
execFileSync.mockReturnValueOnce('image exists');
});
it('should use auto-tagging entrypoint for issue labeling', async () => {
const execFileAsync = promisify(require('child_process').execFile);
execFileAsync.mockResolvedValueOnce({
stdout: 'Applied labels: bug, high-priority',
stderr: ''
});
const result = await processCommand({
repoFullName: 'owner/repo',
issueNumber: 123,
command: 'Auto-tag this issue based on content',
isPullRequest: false,
branchName: null,
operationType: 'auto-tagging'
});
// Should use the tagging-specific entrypoint
const dockerCall = execFileAsync.mock.calls[0];
const dockerArgs = dockerCall[1];
expect(dockerArgs).toContain('/scripts/runtime/claudecode-tagging-entrypoint.sh');
expect(result).toContain('Applied labels');
});
it('should use standard entrypoint for PR reviews', async () => {
const execFileAsync = promisify(require('child_process').execFile);
execFileAsync.mockResolvedValueOnce({
stdout: 'PR review completed with detailed feedback',
stderr: ''
});
const result = await processCommand({
repoFullName: 'owner/repo',
issueNumber: 42,
command: 'Review this PR thoroughly',
isPullRequest: true,
branchName: 'feature/new-functionality'
});
// Should use the standard entrypoint
const dockerCall = execFileAsync.mock.calls[0];
const dockerArgs = dockerCall[1];
expect(dockerArgs).toContain('/usr/local/bin/entrypoint.sh');
expect(result).toContain('PR review completed');
});
});
describe('Container failure recovery', () => {
beforeEach(() => {
// Mock successful Docker image check
execFileSync.mockReturnValueOnce('image exists');
});
it('should retrieve logs when container execution fails', async () => {
const execFileAsync = promisify(require('child_process').execFile);
// Mock container execution failure
const executionError = new Error('Container execution failed');
executionError.code = 125;
execFileAsync.mockRejectedValueOnce(executionError);
// Mock successful log retrieval
execFileSync.mockReturnValueOnce('Error logs: Authentication failed, please check credentials');
const result = await processCommand({
repoFullName: 'owner/repo',
issueNumber: 123,
command: 'analyze repository',
isPullRequest: false,
branchName: null
});
// Should attempt to get logs as fallback
expect(execFileSync).toHaveBeenCalledWith(
'docker',
['logs', expect.stringMatching(/claude-owner-repo-\d+/)],
expect.objectContaining({
encoding: 'utf8',
maxBuffer: 1024 * 1024
})
);
expect(result).toContain('Error logs: Authentication failed');
});
it('should provide meaningful error when both execution and logs fail', async () => {
const execFileAsync = promisify(require('child_process').execFile);
// Mock container execution failure
execFileAsync.mockRejectedValueOnce(new Error('Container execution failed'));
// Mock log retrieval failure
execFileSync.mockImplementationOnce(() => {
throw new Error('Could not retrieve container logs');
});
const result = await processCommand({
repoFullName: 'owner/repo',
issueNumber: 123,
command: 'analyze repository',
isPullRequest: false,
branchName: null
});
expect(result).toMatch(/error occurred while processing.*request/i);
expect(result).toMatch(/please check.*configuration/i);
});
});
describe('GitHub token validation', () => {
it('should work with fine-grained GitHub tokens', async () => {
execFileSync.mockReturnValueOnce('image exists');
const execFileAsync = promisify(require('child_process').execFile);
execFileAsync.mockResolvedValueOnce({
stdout: 'Successfully used fine-grained token',
stderr: ''
});
const result = await processCommand({
repoFullName: 'owner/repo',
issueNumber: 123,
command: 'check repository access',
isPullRequest: false,
branchName: null
});
expect(result).toContain('Successfully used fine-grained token');
});
});
});