Skip to content

Commit 2614be7

Browse files
CopilotCopilot
authored
refactor: extract shared parameterised test factory for log sub-commands (#2563)
* Initial plan * refactor: extract shared test factory for logs-stats and logs-summary Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/abb7b88b-4cdf-4dee-a3dc-fc3831f15d1b --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent 57478ed commit 2614be7

3 files changed

Lines changed: 177 additions & 295 deletions

File tree

‎src/commands/logs-stats.test.ts‎

Lines changed: 11 additions & 145 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44

55
import { statsCommand, StatsCommandOptions } from './logs-stats';
66
import { logger } from '../logger';
7-
import { LogSource } from '../types';
8-
import { createLogCommandTestHarness } from './test-helpers.test-utils';
7+
import { createLogCommandTests, createLogCommandTestHarness } from './test-helpers.test-utils';
98

109
// Mock dependencies
1110
jest.mock('../logs/log-discovery');
@@ -20,152 +19,19 @@ jest.mock('../logger', () => ({
2019
},
2120
}));
2221

23-
describe('logs-stats command', () => {
24-
const harness = createLogCommandTestHarness();
25-
26-
it('should discover and use most recent log source', async () => {
27-
const mockSource: LogSource = {
28-
type: 'preserved',
29-
path: '/tmp/squid-logs-123',
30-
timestamp: Date.now(),
31-
dateStr: new Date().toLocaleString(),
32-
};
33-
34-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([mockSource]);
35-
harness.mockedDiscovery.selectMostRecent.mockReturnValue(mockSource);
36-
harness.mockedAggregator.loadAndAggregate.mockResolvedValue({
37-
totalRequests: 10,
38-
allowedRequests: 8,
39-
deniedRequests: 2,
40-
uniqueDomains: 3,
41-
byDomain: new Map(),
42-
timeRange: { start: 1000, end: 2000 },
43-
});
44-
harness.mockedFormatter.formatStats.mockReturnValue('formatted output');
45-
46-
const options: StatsCommandOptions = {
47-
format: 'pretty',
48-
};
49-
50-
await statsCommand(options);
51-
52-
expect(harness.mockedDiscovery.discoverLogSources).toHaveBeenCalled();
53-
expect(harness.mockedDiscovery.selectMostRecent).toHaveBeenCalled();
54-
expect(harness.mockedAggregator.loadAndAggregate).toHaveBeenCalledWith(mockSource);
55-
expect(harness.mockedFormatter.formatStats).toHaveBeenCalled();
56-
expect(harness.mockConsoleLog).toHaveBeenCalledWith('formatted output');
57-
});
58-
59-
it('should use specified source when provided', async () => {
60-
const mockSource: LogSource = {
61-
type: 'preserved',
62-
path: '/custom/path',
63-
};
64-
65-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([]);
66-
harness.mockedDiscovery.validateSource.mockResolvedValue(mockSource);
67-
harness.mockedAggregator.loadAndAggregate.mockResolvedValue({
68-
totalRequests: 5,
69-
allowedRequests: 5,
70-
deniedRequests: 0,
71-
uniqueDomains: 2,
72-
byDomain: new Map(),
73-
timeRange: null,
74-
});
75-
harness.mockedFormatter.formatStats.mockReturnValue('formatted');
76-
77-
const options: StatsCommandOptions = {
78-
format: 'json',
79-
source: '/custom/path',
80-
};
81-
82-
await statsCommand(options);
83-
84-
expect(harness.mockedDiscovery.validateSource).toHaveBeenCalledWith('/custom/path');
85-
expect(harness.mockedAggregator.loadAndAggregate).toHaveBeenCalledWith(mockSource);
86-
});
87-
88-
it('should exit with error if no sources found', async () => {
89-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([]);
90-
91-
const options: StatsCommandOptions = {
92-
format: 'pretty',
93-
};
94-
95-
await expect(statsCommand(options)).rejects.toThrow('process.exit called');
96-
expect(harness.mockExit).toHaveBeenCalledWith(1);
97-
});
98-
99-
it('should exit with error if specified source is invalid', async () => {
100-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([]);
101-
harness.mockedDiscovery.validateSource.mockRejectedValue(new Error('Source not found'));
22+
createLogCommandTests<StatsCommandOptions>(
23+
'stats',
24+
statsCommand,
25+
'pretty',
26+
(overrides?) => ({ format: 'pretty', ...overrides } as StatsCommandOptions),
27+
);
10228

103-
const options: StatsCommandOptions = {
104-
format: 'pretty',
105-
source: '/invalid/path',
106-
};
107-
108-
await expect(statsCommand(options)).rejects.toThrow('process.exit called');
109-
expect(harness.mockExit).toHaveBeenCalledWith(1);
110-
});
111-
112-
it('should pass correct format to formatter', async () => {
113-
const mockSource: LogSource = { type: 'running', containerName: 'awf-squid' };
114-
115-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([mockSource]);
116-
harness.mockedDiscovery.selectMostRecent.mockReturnValue(mockSource);
117-
harness.mockedAggregator.loadAndAggregate.mockResolvedValue({
118-
totalRequests: 0,
119-
allowedRequests: 0,
120-
deniedRequests: 0,
121-
uniqueDomains: 0,
122-
byDomain: new Map(),
123-
timeRange: null,
124-
});
125-
harness.mockedFormatter.formatStats.mockReturnValue('{}');
126-
127-
await statsCommand({ format: 'json' });
128-
expect(harness.mockedFormatter.formatStats).toHaveBeenCalledWith(
129-
expect.anything(),
130-
'json',
131-
expect.any(Boolean)
132-
);
133-
134-
harness.mockedFormatter.formatStats.mockClear();
135-
await statsCommand({ format: 'markdown' });
136-
expect(harness.mockedFormatter.formatStats).toHaveBeenCalledWith(
137-
expect.anything(),
138-
'markdown',
139-
expect.any(Boolean)
140-
);
141-
142-
harness.mockedFormatter.formatStats.mockClear();
143-
await statsCommand({ format: 'pretty' });
144-
expect(harness.mockedFormatter.formatStats).toHaveBeenCalledWith(
145-
expect.anything(),
146-
'pretty',
147-
expect.any(Boolean)
148-
);
149-
});
150-
151-
it('should handle aggregation errors gracefully', async () => {
152-
const mockSource: LogSource = { type: 'running', containerName: 'awf-squid' };
153-
154-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([mockSource]);
155-
harness.mockedDiscovery.selectMostRecent.mockReturnValue(mockSource);
156-
harness.mockedAggregator.loadAndAggregate.mockRejectedValue(new Error('Failed to load'));
157-
158-
const options: StatsCommandOptions = {
159-
format: 'pretty',
160-
};
161-
162-
await expect(statsCommand(options)).rejects.toThrow('process.exit called');
163-
expect(harness.mockExit).toHaveBeenCalledWith(1);
164-
});
29+
describe('logs-stats command - logging behavior', () => {
30+
const harness = createLogCommandTestHarness();
16531

16632
it('should emit source-selection info logs for non-JSON formats but suppress them for JSON', async () => {
167-
const mockSource: LogSource = {
168-
type: 'preserved',
33+
const mockSource = {
34+
type: 'preserved' as const,
16935
path: '/tmp/squid-logs-123',
17036
dateStr: 'Mon Jan 01 2024',
17137
};

‎src/commands/logs-summary.test.ts‎

Lines changed: 13 additions & 150 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44

55
import { summaryCommand, SummaryCommandOptions } from './logs-summary';
66
import { logger } from '../logger';
7-
import { LogSource } from '../types';
8-
import { createLogCommandTestHarness } from './test-helpers.test-utils';
7+
import { createLogCommandTests, createLogCommandTestHarness } from './test-helpers.test-utils';
98

109
// Mock dependencies
1110
jest.mock('../logs/log-discovery');
@@ -20,44 +19,18 @@ jest.mock('../logger', () => ({
2019
},
2120
}));
2221

23-
describe('logs-summary command', () => {
24-
const harness = createLogCommandTestHarness();
25-
26-
it('should discover and use most recent log source', async () => {
27-
const mockSource: LogSource = {
28-
type: 'preserved',
29-
path: '/tmp/squid-logs-123',
30-
timestamp: Date.now(),
31-
dateStr: new Date().toLocaleString(),
32-
};
33-
34-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([mockSource]);
35-
harness.mockedDiscovery.selectMostRecent.mockReturnValue(mockSource);
36-
harness.mockedAggregator.loadAndAggregate.mockResolvedValue({
37-
totalRequests: 10,
38-
allowedRequests: 8,
39-
deniedRequests: 2,
40-
uniqueDomains: 3,
41-
byDomain: new Map(),
42-
timeRange: { start: 1000, end: 2000 },
43-
});
44-
harness.mockedFormatter.formatStats.mockReturnValue('markdown summary');
22+
createLogCommandTests<SummaryCommandOptions>(
23+
'summary',
24+
summaryCommand,
25+
'markdown',
26+
(overrides?) => ({ format: 'markdown', ...overrides } as SummaryCommandOptions),
27+
);
4528

46-
const options: SummaryCommandOptions = {
47-
format: 'markdown',
48-
};
49-
50-
await summaryCommand(options);
51-
52-
expect(harness.mockedDiscovery.discoverLogSources).toHaveBeenCalled();
53-
expect(harness.mockedDiscovery.selectMostRecent).toHaveBeenCalled();
54-
expect(harness.mockedAggregator.loadAndAggregate).toHaveBeenCalledWith(mockSource);
55-
expect(harness.mockedFormatter.formatStats).toHaveBeenCalled();
56-
expect(harness.mockConsoleLog).toHaveBeenCalledWith('markdown summary');
57-
});
29+
describe('logs-summary command - logging behavior', () => {
30+
const harness = createLogCommandTestHarness();
5831

5932
it('should default to markdown format', async () => {
60-
const mockSource: LogSource = { type: 'running', containerName: 'awf-squid' };
33+
const mockSource = { type: 'running' as const, containerName: 'awf-squid' };
6134

6235
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([mockSource]);
6336
harness.mockedDiscovery.selectMostRecent.mockReturnValue(mockSource);
@@ -77,123 +50,13 @@ describe('logs-summary command', () => {
7750
expect(harness.mockedFormatter.formatStats).toHaveBeenCalledWith(
7851
expect.anything(),
7952
'markdown',
80-
expect.any(Boolean)
53+
expect.any(Boolean),
8154
);
8255
});
8356

84-
it('should use specified source when provided', async () => {
85-
const mockSource: LogSource = {
86-
type: 'preserved',
87-
path: '/custom/path',
88-
};
89-
90-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([]);
91-
harness.mockedDiscovery.validateSource.mockResolvedValue(mockSource);
92-
harness.mockedAggregator.loadAndAggregate.mockResolvedValue({
93-
totalRequests: 5,
94-
allowedRequests: 5,
95-
deniedRequests: 0,
96-
uniqueDomains: 2,
97-
byDomain: new Map(),
98-
timeRange: null,
99-
});
100-
harness.mockedFormatter.formatStats.mockReturnValue('formatted');
101-
102-
const options: SummaryCommandOptions = {
103-
format: 'markdown',
104-
source: '/custom/path',
105-
};
106-
107-
await summaryCommand(options);
108-
109-
expect(harness.mockedDiscovery.validateSource).toHaveBeenCalledWith('/custom/path');
110-
expect(harness.mockedAggregator.loadAndAggregate).toHaveBeenCalledWith(mockSource);
111-
});
112-
113-
it('should exit with error if no sources found', async () => {
114-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([]);
115-
116-
const options: SummaryCommandOptions = {
117-
format: 'markdown',
118-
};
119-
120-
await expect(summaryCommand(options)).rejects.toThrow('process.exit called');
121-
expect(harness.mockExit).toHaveBeenCalledWith(1);
122-
});
123-
124-
it('should exit with error if specified source is invalid', async () => {
125-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([]);
126-
harness.mockedDiscovery.validateSource.mockRejectedValue(new Error('Source not found'));
127-
128-
const options: SummaryCommandOptions = {
129-
format: 'markdown',
130-
source: '/invalid/path',
131-
};
132-
133-
await expect(summaryCommand(options)).rejects.toThrow('process.exit called');
134-
expect(harness.mockExit).toHaveBeenCalledWith(1);
135-
});
136-
137-
it('should support all output formats', async () => {
138-
const mockSource: LogSource = { type: 'running', containerName: 'awf-squid' };
139-
140-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([mockSource]);
141-
harness.mockedDiscovery.selectMostRecent.mockReturnValue(mockSource);
142-
harness.mockedAggregator.loadAndAggregate.mockResolvedValue({
143-
totalRequests: 0,
144-
allowedRequests: 0,
145-
deniedRequests: 0,
146-
uniqueDomains: 0,
147-
byDomain: new Map(),
148-
timeRange: null,
149-
});
150-
harness.mockedFormatter.formatStats.mockReturnValue('output');
151-
152-
// Test JSON format
153-
await summaryCommand({ format: 'json' });
154-
expect(harness.mockedFormatter.formatStats).toHaveBeenCalledWith(
155-
expect.anything(),
156-
'json',
157-
expect.any(Boolean)
158-
);
159-
160-
// Test markdown format
161-
harness.mockedFormatter.formatStats.mockClear();
162-
await summaryCommand({ format: 'markdown' });
163-
expect(harness.mockedFormatter.formatStats).toHaveBeenCalledWith(
164-
expect.anything(),
165-
'markdown',
166-
expect.any(Boolean)
167-
);
168-
169-
// Test pretty format
170-
harness.mockedFormatter.formatStats.mockClear();
171-
await summaryCommand({ format: 'pretty' });
172-
expect(harness.mockedFormatter.formatStats).toHaveBeenCalledWith(
173-
expect.anything(),
174-
'pretty',
175-
expect.any(Boolean)
176-
);
177-
});
178-
179-
it('should handle aggregation errors gracefully', async () => {
180-
const mockSource: LogSource = { type: 'running', containerName: 'awf-squid' };
181-
182-
harness.mockedDiscovery.discoverLogSources.mockResolvedValue([mockSource]);
183-
harness.mockedDiscovery.selectMostRecent.mockReturnValue(mockSource);
184-
harness.mockedAggregator.loadAndAggregate.mockRejectedValue(new Error('Failed to load'));
185-
186-
const options: SummaryCommandOptions = {
187-
format: 'markdown',
188-
};
189-
190-
await expect(summaryCommand(options)).rejects.toThrow('process.exit called');
191-
expect(harness.mockExit).toHaveBeenCalledWith(1);
192-
});
193-
19457
it('should emit source-selection info logs only for pretty format, suppress them for markdown and json', async () => {
195-
const mockSource: LogSource = {
196-
type: 'preserved',
58+
const mockSource = {
59+
type: 'preserved' as const,
19760
path: '/tmp/squid-logs-123',
19861
dateStr: 'Mon Jan 01 2024',
19962
};

0 commit comments

Comments
 (0)