refactor(core): Refactored and removed redundant test lines in telemetry (#12356)

This commit is contained in:
Jainam M
2025-11-04 14:12:31 +05:30
committed by GitHub
parent ab73051298
commit 6ab1b239ca
3 changed files with 288 additions and 422 deletions

View File

@@ -4,7 +4,7 @@
* SPDX-License-Identifier: Apache-2.0
*/
import { vi } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import * as path from 'node:path';
import * as os from 'node:os';
import {
@@ -296,60 +296,52 @@ describe('extensionSettings', () => {
});
describe('promptForSetting', () => {
it('should use prompts with type "password" for sensitive settings', async () => {
const setting: ExtensionSetting = {
name: 'API Key',
description: 'Your secret key',
envVar: 'API_KEY',
sensitive: true,
};
vi.mocked(prompts).mockResolvedValue({ value: 'secret-key' });
it.each([
{
description:
'should use prompts with type "password" for sensitive settings',
setting: {
name: 'API Key',
description: 'Your secret key',
envVar: 'API_KEY',
sensitive: true,
},
expectedType: 'password',
promptValue: 'secret-key',
},
{
description:
'should use prompts with type "text" for non-sensitive settings',
setting: {
name: 'Username',
description: 'Your public username',
envVar: 'USERNAME',
sensitive: false,
},
expectedType: 'text',
promptValue: 'test-user',
},
{
description: 'should default to "text" if sensitive is undefined',
setting: {
name: 'Username',
description: 'Your public username',
envVar: 'USERNAME',
},
expectedType: 'text',
promptValue: 'test-user',
},
])('$description', async ({ setting, expectedType, promptValue }) => {
vi.mocked(prompts).mockResolvedValue({ value: promptValue });
const result = await promptForSetting(setting);
const result = await promptForSetting(setting as ExtensionSetting);
expect(prompts).toHaveBeenCalledWith({
type: 'password',
type: expectedType,
name: 'value',
message: 'API Key\nYour secret key',
message: `${setting.name}\n${setting.description}`,
});
expect(result).toBe('secret-key');
});
it('should use prompts with type "text" for non-sensitive settings', async () => {
const setting: ExtensionSetting = {
name: 'Username',
description: 'Your public username',
envVar: 'USERNAME',
// sensitive: false,
};
vi.mocked(prompts).mockResolvedValue({ value: 'test-user' });
const result = await promptForSetting(setting);
expect(prompts).toHaveBeenCalledWith({
type: 'text',
name: 'value',
message: 'Username\nYour public username',
});
expect(result).toBe('test-user');
});
it('should default to "text" if sensitive is undefined', async () => {
const setting: ExtensionSetting = {
name: 'Username',
description: 'Your public username',
envVar: 'USERNAME',
};
vi.mocked(prompts).mockResolvedValue({ value: 'test-user' });
const result = await promptForSetting(setting);
expect(prompts).toHaveBeenCalledWith({
type: 'text',
name: 'value',
message: 'Username\nYour public username',
});
expect(result).toBe('test-user');
expect(result).toBe(promptValue);
});
});
});

View File

@@ -174,165 +174,138 @@ describe('git extension helpers', () => {
});
});
it('should return NOT_UPDATABLE for non-git extensions', async () => {
const extension: GeminiCLIExtension = {
it.each([
{
testName: 'should return NOT_UPDATABLE for non-git extensions',
extension: {
installMetadata: { type: 'link', source: '' },
},
mockSetup: () => {},
expected: ExtensionUpdateState.NOT_UPDATABLE,
},
{
testName: 'should return ERROR if no remotes found',
extension: {
installMetadata: { type: 'git', source: '' },
},
mockSetup: () => {
mockGit.getRemotes.mockResolvedValue([]);
},
expected: ExtensionUpdateState.ERROR,
},
{
testName:
'should return UPDATE_AVAILABLE when remote hash is different',
extension: {
installMetadata: { type: 'git', source: 'my/ext' },
},
mockSetup: () => {
mockGit.getRemotes.mockResolvedValue([
{ name: 'origin', refs: { fetch: 'http://my-repo.com' } },
]);
mockGit.listRemote.mockResolvedValue('remote-hash\tHEAD');
mockGit.revparse.mockResolvedValue('local-hash');
},
expected: ExtensionUpdateState.UPDATE_AVAILABLE,
},
{
testName:
'should return UP_TO_DATE when remote and local hashes are the same',
extension: {
installMetadata: { type: 'git', source: 'my/ext' },
},
mockSetup: () => {
mockGit.getRemotes.mockResolvedValue([
{ name: 'origin', refs: { fetch: 'http://my-repo.com' } },
]);
mockGit.listRemote.mockResolvedValue('same-hash\tHEAD');
mockGit.revparse.mockResolvedValue('same-hash');
},
expected: ExtensionUpdateState.UP_TO_DATE,
},
{
testName: 'should return ERROR on git error',
extension: {
installMetadata: { type: 'git', source: 'my/ext' },
},
mockSetup: () => {
mockGit.getRemotes.mockRejectedValue(new Error('git error'));
},
expected: ExtensionUpdateState.ERROR,
},
])('$testName', async ({ extension, mockSetup, expected }) => {
const fullExtension: GeminiCLIExtension = {
name: 'test',
id: 'test-id',
path: '/ext',
version: '1.0.0',
isActive: true,
installMetadata: {
type: 'link',
source: '',
},
contextFiles: [],
};
const result = await checkForExtensionUpdate(extension, extensionManager);
expect(result).toBe(ExtensionUpdateState.NOT_UPDATABLE);
});
it('should return ERROR if no remotes found', async () => {
const extension: GeminiCLIExtension = {
name: 'test',
id: 'test-id',
path: '/ext',
version: '1.0.0',
isActive: true,
installMetadata: {
type: 'git',
source: '',
},
contextFiles: [],
};
mockGit.getRemotes.mockResolvedValue([]);
const result = await checkForExtensionUpdate(extension, extensionManager);
expect(result).toBe(ExtensionUpdateState.ERROR);
});
it('should return UPDATE_AVAILABLE when remote hash is different', async () => {
const extension: GeminiCLIExtension = {
name: 'test',
id: 'test-id',
path: '/ext',
version: '1.0.0',
isActive: true,
installMetadata: {
type: 'git',
source: 'my/ext',
},
contextFiles: [],
};
mockGit.getRemotes.mockResolvedValue([
{ name: 'origin', refs: { fetch: 'http://my-repo.com' } },
]);
mockGit.listRemote.mockResolvedValue('remote-hash\tHEAD');
mockGit.revparse.mockResolvedValue('local-hash');
const result = await checkForExtensionUpdate(extension, extensionManager);
expect(result).toBe(ExtensionUpdateState.UPDATE_AVAILABLE);
});
it('should return UP_TO_DATE when remote and local hashes are the same', async () => {
const extension: GeminiCLIExtension = {
name: 'test',
id: 'test-id',
path: '/ext',
version: '1.0.0',
isActive: true,
installMetadata: {
type: 'git',
source: 'my/ext',
},
contextFiles: [],
};
mockGit.getRemotes.mockResolvedValue([
{ name: 'origin', refs: { fetch: 'http://my-repo.com' } },
]);
mockGit.listRemote.mockResolvedValue('same-hash\tHEAD');
mockGit.revparse.mockResolvedValue('same-hash');
const result = await checkForExtensionUpdate(extension, extensionManager);
expect(result).toBe(ExtensionUpdateState.UP_TO_DATE);
});
it('should return ERROR on git error', async () => {
const extension: GeminiCLIExtension = {
name: 'test',
id: 'test-id',
path: '/ext',
version: '1.0.0',
isActive: true,
installMetadata: {
type: 'git',
source: 'my/ext',
},
contextFiles: [],
};
mockGit.getRemotes.mockRejectedValue(new Error('git error'));
const result = await checkForExtensionUpdate(extension, extensionManager);
expect(result).toBe(ExtensionUpdateState.ERROR);
...extension,
} as unknown as GeminiCLIExtension;
mockSetup();
const result = await checkForExtensionUpdate(
fullExtension,
extensionManager,
);
expect(result).toBe(expected);
});
});
describe('fetchReleaseFromGithub', () => {
it('should fetch the latest release if allowPreRelease is true', async () => {
const releases = [{ tag_name: 'v1.0.0-alpha' }, { tag_name: 'v0.9.0' }];
fetchJsonMock.mockResolvedValueOnce(releases);
it.each([
{
ref: undefined,
allowPreRelease: true,
mockedResponse: [{ tag_name: 'v1.0.0-alpha' }, { tag_name: 'v0.9.0' }],
expectedUrl:
'https://api.github.com/repos/owner/repo/releases?per_page=1',
expectedResult: { tag_name: 'v1.0.0-alpha' },
},
{
ref: undefined,
allowPreRelease: false,
mockedResponse: { tag_name: 'v0.9.0' },
expectedUrl: 'https://api.github.com/repos/owner/repo/releases/latest',
expectedResult: { tag_name: 'v0.9.0' },
},
{
ref: 'v0.9.0',
allowPreRelease: undefined,
mockedResponse: { tag_name: 'v0.9.0' },
expectedUrl:
'https://api.github.com/repos/owner/repo/releases/tags/v0.9.0',
expectedResult: { tag_name: 'v0.9.0' },
},
{
ref: undefined,
allowPreRelease: undefined,
mockedResponse: { tag_name: 'v0.9.0' },
expectedUrl: 'https://api.github.com/repos/owner/repo/releases/latest',
expectedResult: { tag_name: 'v0.9.0' },
},
])(
'should fetch release with ref=$ref and allowPreRelease=$allowPreRelease',
async ({
ref,
allowPreRelease,
mockedResponse,
expectedUrl,
expectedResult,
}) => {
fetchJsonMock.mockResolvedValueOnce(mockedResponse);
const result = await fetchReleaseFromGithub(
'owner',
'repo',
undefined,
true,
);
const result = await fetchReleaseFromGithub(
'owner',
'repo',
ref,
allowPreRelease,
);
expect(fetchJsonMock).toHaveBeenCalledWith(
'https://api.github.com/repos/owner/repo/releases?per_page=1',
);
expect(result).toEqual(releases[0]);
});
it('should fetch the latest release if allowPreRelease is false', async () => {
const release = { tag_name: 'v0.9.0' };
fetchJsonMock.mockResolvedValueOnce(release);
const result = await fetchReleaseFromGithub(
'owner',
'repo',
undefined,
false,
);
expect(fetchJsonMock).toHaveBeenCalledWith(
'https://api.github.com/repos/owner/repo/releases/latest',
);
expect(result).toEqual(release);
});
it('should fetch a release by tag if ref is provided', async () => {
const release = { tag_name: 'v0.9.0' };
fetchJsonMock.mockResolvedValueOnce(release);
const result = await fetchReleaseFromGithub('owner', 'repo', 'v0.9.0');
expect(fetchJsonMock).toHaveBeenCalledWith(
'https://api.github.com/repos/owner/repo/releases/tags/v0.9.0',
);
expect(result).toEqual(release);
});
it('should fetch latest stable release if allowPreRelease is undefined', async () => {
const release = { tag_name: 'v0.9.0' };
fetchJsonMock.mockResolvedValueOnce(release);
const result = await fetchReleaseFromGithub('owner', 'repo');
expect(fetchJsonMock).toHaveBeenCalledWith(
'https://api.github.com/repos/owner/repo/releases/latest',
);
expect(result).toEqual(release);
});
expect(fetchJsonMock).toHaveBeenCalledWith(expectedUrl);
expect(result).toEqual(expectedResult);
},
);
});
describe('findReleaseAsset', () => {
@@ -344,31 +317,27 @@ describe('git extension helpers', () => {
{ name: 'extension-generic.tar.gz', browser_download_url: 'url5' },
];
it('should find asset matching platform and architecture', () => {
mockPlatform.mockReturnValue('darwin');
mockArch.mockReturnValue('arm64');
const result = findReleaseAsset(assets);
expect(result).toEqual(assets[0]);
});
it.each([
{ platform: 'darwin', arch: 'arm64', expected: assets[0] },
{ platform: 'linux', arch: 'arm64', expected: assets[2] },
it('should find asset matching platform if arch does not match', () => {
mockPlatform.mockReturnValue('linux');
mockArch.mockReturnValue('arm64');
const result = findReleaseAsset(assets);
expect(result).toEqual(assets[2]);
});
{ platform: 'sunos', arch: 'x64', expected: undefined },
])(
'should find asset matching platform and architecture',
it('should return undefined if no matching asset is found', () => {
mockPlatform.mockReturnValue('sunos');
mockArch.mockReturnValue('x64');
const result = findReleaseAsset(assets);
expect(result).toBeUndefined();
});
({ platform, arch, expected }) => {
mockPlatform.mockReturnValue(platform);
mockArch.mockReturnValue(arch);
const result = findReleaseAsset(assets);
expect(result).toEqual(expected);
},
);
it('should find generic asset if it is the only one', () => {
const singleAsset = [
{ name: 'extension.tar.gz', browser_download_url: 'url' },
];
mockPlatform.mockReturnValue('darwin');
mockArch.mockReturnValue('arm64');
const result = findReleaseAsset(singleAsset);
@@ -380,6 +349,7 @@ describe('git extension helpers', () => {
{ name: 'extension-1.tar.gz', browser_download_url: 'url1' },
{ name: 'extension-2.tar.gz', browser_download_url: 'url2' },
];
mockPlatform.mockReturnValue('darwin');
mockArch.mockReturnValue('arm64');
const result = findReleaseAsset(multipleGenericAssets);
@@ -388,67 +358,54 @@ describe('git extension helpers', () => {
});
describe('parseGitHubRepoForReleases', () => {
it('should parse owner and repo from a full GitHub URL', () => {
const source = 'https://github.com/owner/repo.git';
const { owner, repo } = tryParseGithubUrl(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});
it('should parse owner and repo from a full GitHub URL without .git', () => {
const source = 'https://github.com/owner/repo';
const { owner, repo } = tryParseGithubUrl(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});
it('should parse owner and repo from a full GitHub URL with a trailing slash', () => {
const source = 'https://github.com/owner/repo/';
const { owner, repo } = tryParseGithubUrl(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});
it('should parse owner and repo from a GitHub SSH URL', () => {
const source = 'git@github.com:owner/repo.git';
const { owner, repo } = tryParseGithubUrl(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});
it.each([
{
source: 'https://github.com/owner/repo.git',
owner: 'owner',
repo: 'repo',
},
{
source: 'https://github.com/owner/repo',
owner: 'owner',
repo: 'repo',
},
{
source: 'https://github.com/owner/repo/',
owner: 'owner',
repo: 'repo',
},
{
source: 'git@github.com:owner/repo.git',
owner: 'owner',
repo: 'repo',
},
{ source: 'owner/repo', owner: 'owner', repo: 'repo' },
{ source: 'owner/repo.git', owner: 'owner', repo: 'repo' },
])(
'should parse owner and repo from $source',
({ source, owner, repo }) => {
const result = tryParseGithubUrl(source)!;
expect(result.owner).toBe(owner);
expect(result.repo).toBe(repo);
},
);
it('should return null on a non-GitHub URL', () => {
const source = 'https://example.com/owner/repo.git';
expect(tryParseGithubUrl(source)).toBe(null);
});
it('should parse owner and repo from a shorthand string', () => {
const source = 'owner/repo';
const { owner, repo } = tryParseGithubUrl(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});
it('should handle .git suffix in repo name', () => {
const source = 'owner/repo.git';
const { owner, repo } = tryParseGithubUrl(source)!;
expect(owner).toBe('owner');
expect(repo).toBe('repo');
});
it('should throw error for invalid source format', () => {
const source = 'invalid-format';
expect(() => tryParseGithubUrl(source)).toThrow(
'Invalid GitHub repository source: invalid-format. Expected "owner/repo" or a github repo uri.',
);
});
it('should throw error for source with too many parts', () => {
const source = 'https://github.com/owner/repo/extra';
expect(() => tryParseGithubUrl(source)).toThrow(
'Invalid GitHub repository source: https://github.com/owner/repo/extra. Expected "owner/repo" or a github repo uri.',
);
});
it.each([
{ source: 'invalid-format' },
{ source: 'https://github.com/owner/repo/extra' },
])(
'should throw error for invalid source format: $source',
({ source }) => {
expect(() => tryParseGithubUrl(source)).toThrow(
`Invalid GitHub repository source: ${source}. Expected "owner/repo" or a github repo uri.`,
);
},
);
});
describe('extractFile', () => {

View File

@@ -280,96 +280,33 @@ describe('Telemetry Metrics', () => {
expect(mockCounterAddFn).not.toHaveBeenCalled();
});
it('should record token usage with the correct attributes', () => {
initializeMetricsModule(mockConfig);
recordTokenUsageMetricsModule(mockConfig, 100, {
model: 'gemini-pro',
type: 'input',
});
expect(mockCounterAddFn).toHaveBeenCalledTimes(2);
expect(mockCounterAddFn).toHaveBeenNthCalledWith(1, 1, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
});
expect(mockCounterAddFn).toHaveBeenNthCalledWith(2, 100, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
model: 'gemini-pro',
type: 'input',
});
});
it.each([
{ type: 'input', tokens: 100, model: 'gemini-pro' },
{ type: 'output', tokens: 50, model: 'gemini-pro' },
{ type: 'thought', tokens: 25, model: 'gemini-pro' },
{ type: 'cache', tokens: 75, model: 'gemini-pro' },
{ type: 'tool', tokens: 125, model: 'gemini-pro' },
{ type: 'input', tokens: 200, model: 'gemini-different-model' },
])(
'should record token usage for $type type with $tokens tokens for model $model',
({ type, tokens, model }) => {
initializeMetricsModule(mockConfig);
mockCounterAddFn.mockClear();
it('should record token usage for different types', () => {
initializeMetricsModule(mockConfig);
mockCounterAddFn.mockClear();
recordTokenUsageMetricsModule(mockConfig, tokens, {
model,
type: type as 'input' | 'output' | 'thought' | 'cache' | 'tool',
});
recordTokenUsageMetricsModule(mockConfig, 50, {
model: 'gemini-pro',
type: 'output',
});
expect(mockCounterAddFn).toHaveBeenCalledWith(50, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
model: 'gemini-pro',
type: 'output',
});
recordTokenUsageMetricsModule(mockConfig, 25, {
model: 'gemini-pro',
type: 'thought',
});
expect(mockCounterAddFn).toHaveBeenCalledWith(25, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
model: 'gemini-pro',
type: 'thought',
});
recordTokenUsageMetricsModule(mockConfig, 75, {
model: 'gemini-pro',
type: 'cache',
});
expect(mockCounterAddFn).toHaveBeenCalledWith(75, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
model: 'gemini-pro',
type: 'cache',
});
recordTokenUsageMetricsModule(mockConfig, 125, {
model: 'gemini-pro',
type: 'tool',
});
expect(mockCounterAddFn).toHaveBeenCalledWith(125, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
model: 'gemini-pro',
type: 'tool',
});
});
it('should handle different models', () => {
initializeMetricsModule(mockConfig);
mockCounterAddFn.mockClear();
recordTokenUsageMetricsModule(mockConfig, 200, {
model: 'gemini-different-model',
type: 'input',
});
expect(mockCounterAddFn).toHaveBeenCalledWith(200, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
model: 'gemini-different-model',
type: 'input',
});
});
expect(mockCounterAddFn).toHaveBeenCalledWith(tokens, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
model,
type,
});
},
);
});
describe('recordLinesChanged metric', () => {
@@ -921,80 +858,60 @@ describe('Telemetry Metrics', () => {
});
describe('recordMemoryUsage', () => {
it('should record memory usage for different memory types', () => {
initializeMetricsModule(mockConfig);
mockHistogramRecordFn.mockClear();
recordMemoryUsageModule(mockConfig, 15728640, {
it.each([
{
memory_type: MemoryMetricType.HEAP_USED,
component: 'startup',
});
expect(mockHistogramRecordFn).toHaveBeenCalledWith(15728640, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
memory_type: 'heap_used',
component: 'startup',
});
});
it('should record memory usage for all memory metric types', () => {
initializeMetricsModule(mockConfig);
mockHistogramRecordFn.mockClear();
recordMemoryUsageModule(mockConfig, 31457280, {
value: 15728640,
},
{
memory_type: MemoryMetricType.HEAP_TOTAL,
component: 'api_call',
});
recordMemoryUsageModule(mockConfig, 2097152, {
value: 31457280,
},
{
memory_type: MemoryMetricType.EXTERNAL,
component: 'tool_execution',
});
recordMemoryUsageModule(mockConfig, 41943040, {
value: 2097152,
},
{
memory_type: MemoryMetricType.RSS,
component: 'memory_monitor',
});
expect(mockHistogramRecordFn).toHaveBeenCalledTimes(3); // One for each call
expect(mockHistogramRecordFn).toHaveBeenNthCalledWith(1, 31457280, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
memory_type: 'heap_total',
component: 'api_call',
});
expect(mockHistogramRecordFn).toHaveBeenNthCalledWith(2, 2097152, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
memory_type: 'external',
component: 'tool_execution',
});
expect(mockHistogramRecordFn).toHaveBeenNthCalledWith(3, 41943040, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
memory_type: 'rss',
component: 'memory_monitor',
});
});
it('should record memory usage without component', () => {
initializeMetricsModule(mockConfig);
mockHistogramRecordFn.mockClear();
recordMemoryUsageModule(mockConfig, 15728640, {
value: 41943040,
},
{
memory_type: MemoryMetricType.HEAP_USED,
});
component: undefined,
value: 15728640,
},
])(
'should record memory usage for $memory_type',
({ memory_type, component, value }) => {
initializeMetricsModule(mockConfig);
mockHistogramRecordFn.mockClear();
expect(mockHistogramRecordFn).toHaveBeenCalledWith(15728640, {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
memory_type: 'heap_used',
});
});
recordMemoryUsageModule(mockConfig, value, {
memory_type,
component,
});
const expectedAttributes: Record<string, unknown> = {
'session.id': 'test-session-id',
'installation.id': 'test-installation-id',
'user.email': 'test@example.com',
memory_type,
};
if (component) {
expectedAttributes['component'] = component;
}
expect(mockHistogramRecordFn).toHaveBeenCalledWith(
value,
expectedAttributes,
);
},
);
});
describe('recordCpuUsage', () => {