Skip to content

Commit 111eab8

Browse files
committed
fix(extension): validate threshold config values as numbers; guard updateStatusBar in rate-limit handler
getConfig: coerce threshold.warning/critical to Number with isFinite fallback so non-numeric user config values (e.g. strings) don't produce NaN in comparisons, silently breaking color indicators. updateStatusBar in RATE_LIMIT catch block: wrap in try/catch so a failure there doesn't escape as an unhandled promise rejection. Also add tests/extension.test.js (14 tests) covering: - formatTimestamp: same-day vs different-day, zero-padding - getConfig: numeric coercion, NaN fallback, warning > critical clamping - buildTooltip: unlimited, rate-limited, overage conditions Introduce tests/setup.js (Module._resolveFilename patch) + tests/__mocks__/vscode.js to redirect require('vscode') to a local mock for the Node test environment.
1 parent 54de060 commit 111eab8

5 files changed

Lines changed: 234 additions & 4 deletions

File tree

src/extension.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,11 @@ async function refresh(promptSignIn = false, isManual = false) {
102102
} else if (code === 'RATE_LIMIT') {
103103
if (lastData) {
104104
// Keep last data but mark tooltip
105-
updateStatusBar(lastData, true);
105+
try {
106+
updateStatusBar(lastData, true);
107+
} catch {
108+
showError('Rate limited');
109+
}
106110
} else {
107111
showError('Rate limited');
108112
}
@@ -274,8 +278,11 @@ function clearTimer() {
274278

275279
function getConfig() {
276280
const cfg = vscode.workspace.getConfiguration('githubCopilotUsage');
277-
const warning = cfg.get('threshold.warning', 75);
278-
const critical = cfg.get('threshold.critical', 90);
281+
const rawWarning = cfg.get('threshold.warning', 75);
282+
const rawCritical = cfg.get('threshold.critical', 90);
283+
// Coerce to number and fall back to defaults if non-numeric (e.g. user entered a string)
284+
const warning = Number.isFinite(Number(rawWarning)) ? Number(rawWarning) : 75;
285+
const critical = Number.isFinite(Number(rawCritical)) ? Number(rawCritical) : 90;
279286
return {
280287
refreshIntervalMinutes: cfg.get('refreshIntervalMinutes', 5),
281288
thresholdEnabled: cfg.get('threshold.enabled', true),
@@ -285,4 +292,4 @@ function getConfig() {
285292
};
286293
}
287294

288-
module.exports = { activate, deactivate };
295+
module.exports = { activate, deactivate, formatTimestamp, getConfig, buildTooltip };

tests/__mocks__/vscode.js

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
'use strict';
2+
3+
/**
4+
* Minimal vscode mock for Vitest.
5+
* No vi.fn() usage — this file loads in a plain Node context before test globals.
6+
*/
7+
8+
class MarkdownString {
9+
constructor(value = '') {
10+
this.value = value;
11+
this.isTrusted = false;
12+
}
13+
appendText(text) {
14+
this.value += text;
15+
return this;
16+
}
17+
appendMarkdown(md) {
18+
this.value += md;
19+
return this;
20+
}
21+
}
22+
23+
class ThemeColor {
24+
constructor(id) {
25+
this.id = id;
26+
}
27+
}
28+
29+
const StatusBarAlignment = { Left: 1, Right: 2 };
30+
31+
const workspace = {
32+
getConfiguration: () => ({ get: (_key, defaultValue) => defaultValue }),
33+
onDidChangeConfiguration: () => ({ dispose: () => {} }),
34+
};
35+
36+
const window = {
37+
createStatusBarItem: () => ({
38+
show: () => {},
39+
hide: () => {},
40+
dispose: () => {},
41+
text: '',
42+
tooltip: undefined,
43+
color: undefined,
44+
command: undefined,
45+
}),
46+
};
47+
48+
const authentication = {
49+
getSession: () => Promise.resolve(null),
50+
};
51+
52+
const commands = {
53+
registerCommand: () => ({ dispose: () => {} }),
54+
};
55+
56+
const extensions = {
57+
getExtension: () => null,
58+
};
59+
60+
module.exports = {
61+
MarkdownString,
62+
ThemeColor,
63+
StatusBarAlignment,
64+
workspace,
65+
window,
66+
authentication,
67+
commands,
68+
extensions,
69+
};

tests/extension.test.js

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
'use strict';
2+
3+
const { formatTimestamp, getConfig, buildTooltip } = require('../src/extension');
4+
5+
// ---------------------------------------------------------------------------
6+
// formatTimestamp
7+
// ---------------------------------------------------------------------------
8+
9+
describe('formatTimestamp', () => {
10+
it('returns HH:mm:ss only when the date is today', () => {
11+
const now = new Date();
12+
const result = formatTimestamp(now);
13+
// Should match HH:mm:ss (no date prefix)
14+
expect(result).toMatch(/^\d{2}:\d{2}:\d{2}$/);
15+
});
16+
17+
it('returns YYYY-MM-DD HH:mm:ss when the date is a different day', () => {
18+
const past = new Date('2025-01-15T09:05:03');
19+
const result = formatTimestamp(past);
20+
expect(result).toBe('2025-01-15 09:05:03');
21+
});
22+
23+
it('zero-pads single-digit hours, minutes, seconds', () => {
24+
const past = new Date('2024-03-07T01:02:03');
25+
const result = formatTimestamp(past);
26+
expect(result).toBe('2024-03-07 01:02:03');
27+
});
28+
});
29+
30+
// ---------------------------------------------------------------------------
31+
// getConfig – numeric coercion and clamping (tests the NaN fix)
32+
// ---------------------------------------------------------------------------
33+
34+
describe('getConfig', () => {
35+
function mockCfg(values) {
36+
const vscode = require('vscode');
37+
vscode.workspace.getConfiguration = () => ({
38+
get: (key, defaultValue) => (key in values ? values[key] : defaultValue),
39+
});
40+
}
41+
42+
afterEach(() => {
43+
// Restore default: always returns the default value
44+
const vscode = require('vscode');
45+
vscode.workspace.getConfiguration = () => ({ get: (_key, def) => def });
46+
});
47+
48+
it('returns numeric thresholds from valid config', () => {
49+
mockCfg({ 'threshold.warning': 60, 'threshold.critical': 85 });
50+
const cfg = getConfig();
51+
expect(cfg.thresholdWarning).toBe(60);
52+
expect(cfg.thresholdCritical).toBe(85);
53+
});
54+
55+
it('falls back to defaults when threshold values are non-numeric strings', () => {
56+
mockCfg({ 'threshold.warning': 'off', 'threshold.critical': 'off' });
57+
const cfg = getConfig();
58+
// Both fall back to defaults (75, 90)
59+
expect(cfg.thresholdWarning).toBe(75);
60+
expect(cfg.thresholdCritical).toBe(90);
61+
});
62+
63+
it('falls back to default 90 for critical when only critical is a string', () => {
64+
mockCfg({ 'threshold.warning': 70, 'threshold.critical': 'invalid' });
65+
const cfg = getConfig();
66+
expect(cfg.thresholdCritical).toBe(90);
67+
expect(cfg.thresholdWarning).toBe(70); // valid warning, and 70 < 90
68+
});
69+
70+
it('clamps warning to at most critical when warning > critical', () => {
71+
mockCfg({ 'threshold.warning': 95, 'threshold.critical': 80 });
72+
const cfg = getConfig();
73+
expect(cfg.thresholdWarning).toBe(80); // clamped to critical
74+
expect(cfg.thresholdCritical).toBe(80);
75+
});
76+
77+
it('accepts numeric-string values by coercing them', () => {
78+
mockCfg({ 'threshold.warning': '60', 'threshold.critical': '85' });
79+
const cfg = getConfig();
80+
expect(cfg.thresholdWarning).toBe(60);
81+
expect(cfg.thresholdCritical).toBe(85);
82+
});
83+
});
84+
85+
// ---------------------------------------------------------------------------
86+
// buildTooltip
87+
// ---------------------------------------------------------------------------
88+
89+
describe('buildTooltip', () => {
90+
const BASE_DATA = {
91+
plan: 'Pro',
92+
unlimited: false,
93+
noData: false,
94+
used: 90,
95+
quota: 300,
96+
usedPct: 30,
97+
overageEnabled: false,
98+
overageUsed: 0,
99+
resetDate: new Date('2026-04-01T00:00:00Z'),
100+
};
101+
102+
it('shows plan name and used/quota/pct for normal quota', () => {
103+
const md = buildTooltip(BASE_DATA, false);
104+
expect(md.value).toContain('Pro');
105+
expect(md.value).toContain('90 / 300 (30%)');
106+
});
107+
108+
it('shows Unlimited for unlimited plan', () => {
109+
const md = buildTooltip({ ...BASE_DATA, unlimited: true }, false);
110+
expect(md.value).toContain('Unlimited');
111+
});
112+
113+
it('includes rate-limit notice when isRateLimited is true', () => {
114+
const md = buildTooltip(BASE_DATA, true);
115+
expect(md.value).toContain('Rate limited');
116+
});
117+
118+
it('does not include rate-limit notice when isRateLimited is false', () => {
119+
const md = buildTooltip(BASE_DATA, false);
120+
expect(md.value).not.toContain('Rate limited');
121+
});
122+
123+
it('includes overage count when overageEnabled and overageUsed > 0', () => {
124+
const data = { ...BASE_DATA, overageEnabled: true, overageUsed: 5 };
125+
const md = buildTooltip(data, false);
126+
expect(md.value).toContain('Overage: 5 requests');
127+
});
128+
129+
it('omits overage section when overageUsed is 0', () => {
130+
const md = buildTooltip(BASE_DATA, false);
131+
expect(md.value).not.toContain('Overage');
132+
});
133+
});

tests/setup.js

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
'use strict';
2+
3+
/**
4+
* Redirect `require('vscode')` to the test mock before any test module loads.
5+
* This patches Node's module resolver so that all require('vscode') calls in
6+
* source files and test files resolve to our local mock during testing.
7+
*/
8+
9+
const Module = require('module');
10+
const path = require('path');
11+
12+
const mockPath = path.resolve(__dirname, '__mocks__/vscode.js');
13+
const originalResolveFilename = Module._resolveFilename.bind(Module);
14+
15+
Module._resolveFilename = function (request, ...args) {
16+
if (request === 'vscode') {
17+
return mockPath;
18+
}
19+
return originalResolveFilename(request, ...args);
20+
};

vitest.config.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,6 @@ export default defineConfig({
44
test: {
55
globals: true,
66
environment: 'node',
7+
setupFiles: ['./tests/setup.js'],
78
},
89
});

0 commit comments

Comments
 (0)