From 23987646f0bba6031b3971cf322f76f0bc36b041 Mon Sep 17 00:00:00 2001 From: Iaroslav Omelianenko Date: Wed, 8 Apr 2026 16:29:09 +0300 Subject: [PATCH 1/5] [OPIK-5696] Add `saveToOpikConfigStep` to save configuration to `~/.opik.config` in the Node.js SDK - Introduced `saveToOpikConfigStep` to manage Opik configuration as an INI file with merging support. - Integrated `saveToOpikConfigStep` into the Node.js configuration wizard. - Updated step exports for inclusion in the configuration flow. --- .../opik/configure/src/nodejs/node-wizard.ts | 11 ++ .../src/opik/configure/src/steps/index.ts | 1 + .../src/steps/save-to-opik-config.ts | 118 ++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts diff --git a/sdks/typescript/src/opik/configure/src/nodejs/node-wizard.ts b/sdks/typescript/src/opik/configure/src/nodejs/node-wizard.ts index 8e36cb160d9..d78b71bffb2 100644 --- a/sdks/typescript/src/opik/configure/src/nodejs/node-wizard.ts +++ b/sdks/typescript/src/opik/configure/src/nodejs/node-wizard.ts @@ -19,6 +19,7 @@ import { getOutroMessage } from '../lib/messages'; import { addOrUpdateEnvironmentVariablesStep, runPrettierStep, + saveToOpikConfigStep, } from '../steps/index'; import { uploadEnvironmentVariablesStep } from '../steps/upload-environment-variables/index'; import { buildOpikApiUrl } from '../utils/urls'; @@ -153,6 +154,16 @@ export async function runNodejsWizard(options: WizardOptions): Promise { }); debug(`Environment variables added to ${relativeEnvFilePath}`); + debug('Saving configuration to ~/.opik.config'); + await saveToOpikConfigStep({ + projectName, + urlOverride: buildOpikApiUrl(host), + ...(isLocalDeployment + ? {} + : { apiKey: projectApiKey, workspace: workspaceName }), + }); + debug('Configuration saved to ~/.opik.config'); + analytics.capture('environment variables configured', { envFilePath: relativeEnvFilePath, addedEnvVariables, diff --git a/sdks/typescript/src/opik/configure/src/steps/index.ts b/sdks/typescript/src/opik/configure/src/steps/index.ts index 1621e65844d..c8457e64a3c 100644 --- a/sdks/typescript/src/opik/configure/src/steps/index.ts +++ b/sdks/typescript/src/opik/configure/src/steps/index.ts @@ -2,3 +2,4 @@ export * from './add-editor-rules'; export * from './run-prettier'; export * from './add-or-update-environment-variables'; export * from './upload-environment-variables/index'; +export * from './save-to-opik-config'; diff --git a/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts b/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts new file mode 100644 index 00000000000..4260bd5b055 --- /dev/null +++ b/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts @@ -0,0 +1,118 @@ +import chalk from 'chalk'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import clack from '../utils/clack'; + +const OPIK_CONFIG_FILE = path.join(os.homedir(), '.opik.config'); + +/** Keys we read/write in the [opik] section */ +interface OpikIniSection { + api_key?: string; + url_override?: string; + workspace?: string; + project_name?: string; +} + +/** Parse the [opik] section from a raw INI string (no external deps). */ +function parseOpikSection(content: string): OpikIniSection { + const result: OpikIniSection = {}; + let inOpikSection = false; + + for (const raw of content.split('\n')) { + const line = raw.trim(); + + if (line === '[opik]') { + inOpikSection = true; + continue; + } + + if (line.startsWith('[')) { + inOpikSection = false; + continue; + } + + if (!inOpikSection || !line || line.startsWith('#') || line.startsWith(';')) { + continue; + } + + const eqIdx = line.indexOf('='); + if (eqIdx === -1) continue; + + const key = line.slice(0, eqIdx).trim() as keyof OpikIniSection; + (result as Record)[key] = line.slice(eqIdx + 1).trim().replace(/^["']|["']$/g, ''); + } + + return result; +} + +/** Serialise an OpikIniSection back to an INI string with an [opik] section. */ +function serializeOpikSection( + section: OpikIniSection, + existingContent: string, +): string { + const opikLines = ['[opik]']; + for (const [key, value] of Object.entries(section)) { + if (value !== undefined && value !== '') { + opikLines.push(`${key} = ${value}`); + } + } + const opikBlock = opikLines.join('\n'); + + // Replace existing [opik] block if present, otherwise append + const opikSectionRegex = /\[opik][^[]*/s; + if (opikSectionRegex.test(existingContent)) { + return existingContent.replace(opikSectionRegex, opikBlock + '\n'); + } + + const trimmed = existingContent.trimEnd(); + return trimmed ? `${trimmed}\n\n${opikBlock}\n` : `${opikBlock}\n`; +} + +export interface SaveToOpikConfigOptions { + projectName: string; + urlOverride: string; + apiKey?: string; + workspace?: string; +} + +/** + * Write Opik configuration values to ~/.opik.config (INI format). + * Merges with any existing content so unrelated sections are preserved. + */ +export async function saveToOpikConfigStep( + options: SaveToOpikConfigOptions, +): Promise { + const { projectName, urlOverride, apiKey, workspace } = options; + + try { + let existingContent = ''; + if (fs.existsSync(OPIK_CONFIG_FILE)) { + existingContent = fs.readFileSync(OPIK_CONFIG_FILE, 'utf8'); + } + + const existing = parseOpikSection(existingContent); + + const merged: OpikIniSection = { + ...existing, + url_override: urlOverride, + project_name: projectName, + ...(apiKey ? { api_key: apiKey } : {}), + ...(workspace ? { workspace } : {}), + }; + + const newContent = serializeOpikSection(merged, existingContent); + await fs.promises.writeFile(OPIK_CONFIG_FILE, newContent, { + encoding: 'utf8', + flag: 'w', + }); + + clack.log.success( + `Saved Opik configuration to ${chalk.bold.cyan('~/.opik.config')}`, + ); + } catch (error) { + clack.log.warning( + `Failed to save configuration to ${chalk.bold.cyan('~/.opik.config')}: ${error instanceof Error ? error.message : String(error)}`, + ); + } +} \ No newline at end of file From 9f5f5872ee5452cd184c055934e8a0c3f43766ed Mon Sep 17 00:00:00 2001 From: Iaroslav Omelianenko Date: Wed, 8 Apr 2026 16:59:44 +0300 Subject: [PATCH 2/5] [OPIK-5696] Add unit testing support via Vitest for Node.js SDK configuration tool - Introduced Vitest for testing the Node.js SDK configuration tool. - Exported `parseOpikSection` and `serializeOpikSection` for unit testing. - Added comprehensive test cases for `parseOpikSection`, `serializeOpikSection`, and `saveToOpikConfigStep` logic. - Updated `tsconfig.json` to include `tests` directory. - Added new test scripts to `package.json` and updated workflows to run tests for the configuration tool. --- .../workflows/typescript_sdk_unit_tests.yml | 8 + .../src/opik/configure/package.json | 5 +- .../src/steps/save-to-opik-config.ts | 4 +- .../tests/steps/save-to-opik-config.test.ts | 204 ++++++++++++++++++ .../src/opik/configure/tsconfig.json | 2 +- .../src/opik/configure/vitest.config.ts | 9 + 6 files changed, 228 insertions(+), 4 deletions(-) create mode 100644 sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts create mode 100644 sdks/typescript/src/opik/configure/vitest.config.ts diff --git a/.github/workflows/typescript_sdk_unit_tests.yml b/.github/workflows/typescript_sdk_unit_tests.yml index dfd8915f8e0..744e22f0926 100644 --- a/.github/workflows/typescript_sdk_unit_tests.yml +++ b/.github/workflows/typescript_sdk_unit_tests.yml @@ -42,3 +42,11 @@ jobs: - name: Run tests (npm) run: npm run test + + - name: Install 'configure' tool dependencies + working-directory: sdks/typescript/src/opik/configure + run: npm install + + - name: Run 'configure' tool tests + working-directory: sdks/typescript/src/opik/configure + run: npm run test diff --git a/sdks/typescript/src/opik/configure/package.json b/sdks/typescript/src/opik/configure/package.json index 6483e62228a..9f66fc8fa52 100644 --- a/sdks/typescript/src/opik/configure/package.json +++ b/sdks/typescript/src/opik/configure/package.json @@ -67,6 +67,7 @@ "@types/yargs": "^17.0.35", "dotenv": "^17.2.3", "eslint": "^9.39.2", + "vitest": "^3.0.5", "globals": "^17.3.0", "prettier": "^3.8.1", "tsup": "^8.3.6", @@ -90,7 +91,9 @@ "fix:eslint": "eslint '**/*.{ts,tsx}' --fix", "fmt:check": "prettier --check \"src/**/*.ts\"", "try": "tsx bin.ts", - "typecheck": "tsc --noEmit" + "typecheck": "tsc --noEmit", + "test": "vitest run", + "test:watch": "vitest" }, "author": "Opik", "license": "Apache-2.0", diff --git a/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts b/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts index 4260bd5b055..776c45ac6f2 100644 --- a/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts +++ b/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts @@ -15,7 +15,7 @@ interface OpikIniSection { } /** Parse the [opik] section from a raw INI string (no external deps). */ -function parseOpikSection(content: string): OpikIniSection { +export function parseOpikSection(content: string): OpikIniSection { const result: OpikIniSection = {}; let inOpikSection = false; @@ -47,7 +47,7 @@ function parseOpikSection(content: string): OpikIniSection { } /** Serialise an OpikIniSection back to an INI string with an [opik] section. */ -function serializeOpikSection( +export function serializeOpikSection( section: OpikIniSection, existingContent: string, ): string { diff --git a/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts b/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts new file mode 100644 index 00000000000..d3a154b03f0 --- /dev/null +++ b/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts @@ -0,0 +1,204 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import { + parseOpikSection, + serializeOpikSection, + saveToOpikConfigStep, +} from '../../src/steps'; + +// ESM requires module-level mocking for native node modules. +vi.mock('fs', async () => { + const actual = await vi.importActual('fs'); + return { + ...actual, + existsSync: vi.fn(), + readFileSync: vi.fn(), + promises: { + ...actual.promises, + writeFile: vi.fn(), + }, + }; +}); + +// Silence clack output during tests. +vi.mock('../../src/utils/clack', () => ({ + default: { log: { success: vi.fn(), warning: vi.fn() } }, +})); + +// --------------------------------------------------------------------------- +// parseOpikSection +// --------------------------------------------------------------------------- + +describe('parseOpikSection', () => { + it('parses all keys from an [opik] section', () => { + const content = ` +[opik] +api_key = my-api-key +url_override = https://example.com/api +workspace = my-workspace +project_name = my-project +`.trim(); + + expect(parseOpikSection(content)).toEqual({ + api_key: 'my-api-key', + url_override: 'https://example.com/api', + workspace: 'my-workspace', + project_name: 'my-project', + }); + }); + + it('strips surrounding quotes from values', () => { + const content = `[opik]\napi_key = "quoted-key"\nproject_name = 'single-quoted'`; + const result = parseOpikSection(content); + expect(result.api_key).toBe('quoted-key'); + expect(result.project_name).toBe('single-quoted'); + }); + + it('returns empty object when [opik] section is absent', () => { + expect(parseOpikSection('')).toEqual({}); + expect(parseOpikSection('[other]\nfoo = bar')).toEqual({}); + }); + + it('stops reading at the next section header', () => { + const content = `[opik]\nproject_name = proj\n[other]\napi_key = should-not-appear`; + const result = parseOpikSection(content); + expect(result.project_name).toBe('proj'); + expect(result.api_key).toBeUndefined(); + }); + + it('ignores comment lines', () => { + const content = `[opik]\n# this is a comment\n; another comment\nproject_name = proj`; + expect(parseOpikSection(content).project_name).toBe('proj'); + }); + + it('handles keys without values gracefully (no = sign)', () => { + const content = `[opik]\nbadline\nproject_name = proj`; + expect(parseOpikSection(content).project_name).toBe('proj'); + }); +}); + +// --------------------------------------------------------------------------- +// serializeOpikSection +// --------------------------------------------------------------------------- + +describe('serializeOpikSection', () => { + it('creates a new [opik] block when file is empty', () => { + const result = serializeOpikSection( + { project_name: 'proj', url_override: 'http://localhost/api' }, + '', + ); + expect(result).toContain('[opik]'); + expect(result).toContain('project_name = proj'); + expect(result).toContain('url_override = http://localhost/api'); + }); + + it('replaces an existing [opik] block in-place', () => { + const existing = `[opik]\nproject_name = old\nurl_override = http://old/api\n`; + const result = serializeOpikSection( + { project_name: 'new', url_override: 'http://new/api' }, + existing, + ); + expect(result).toContain('project_name = new'); + expect(result).toContain('url_override = http://new/api'); + expect(result).not.toContain('project_name = old'); + }); + + it('preserves other sections when replacing [opik]', () => { + const existing = `[other]\nfoo = bar\n\n[opik]\nproject_name = old\n`; + const result = serializeOpikSection({ project_name: 'new' }, existing); + expect(result).toContain('[other]'); + expect(result).toContain('foo = bar'); + expect(result).toContain('project_name = new'); + expect(result).not.toContain('project_name = old'); + }); + + it('appends [opik] when other sections exist but no [opik]', () => { + const existing = `[other]\nfoo = bar\n`; + const result = serializeOpikSection({ project_name: 'proj' }, existing); + expect(result).toContain('[other]'); + expect(result).toContain('[opik]'); + expect(result).toContain('project_name = proj'); + }); + + it('omits keys whose value is empty or undefined', () => { + const result = serializeOpikSection( + { project_name: 'proj', api_key: '' }, + '', + ); + expect(result).not.toContain('api_key'); + }); +}); + +// --------------------------------------------------------------------------- +// saveToOpikConfigStep +// --------------------------------------------------------------------------- + +describe('saveToOpikConfigStep', () => { + beforeEach(() => { + vi.mocked(fs.existsSync).mockReturnValue(false); + vi.mocked(fs.readFileSync).mockReturnValue(''); + vi.mocked(fs.promises.writeFile).mockResolvedValue(undefined); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('creates the config file when it does not exist', async () => { + await saveToOpikConfigStep({ + projectName: 'my-project', + urlOverride: 'http://localhost/api', + }); + + expect(fs.promises.writeFile).toHaveBeenCalledOnce(); + const written = vi.mocked(fs.promises.writeFile).mock.calls[0][1] as string; + expect(written).toContain('[opik]'); + expect(written).toContain('project_name = my-project'); + expect(written).toContain('url_override = http://localhost/api'); + expect(written).not.toContain('api_key'); + expect(written).not.toContain('workspace'); + }); + + it('includes api_key and workspace for cloud deployments', async () => { + await saveToOpikConfigStep({ + projectName: 'cloud-proj', + urlOverride: 'https://www.comet.com/opik/api', + apiKey: 'secret-key', + workspace: 'my-workspace', + }); + + expect(fs.promises.writeFile).toHaveBeenCalledOnce(); + const written = vi.mocked(fs.promises.writeFile).mock.calls[0][1] as string; + expect(written).toContain('api_key = secret-key'); + expect(written).toContain('workspace = my-workspace'); + }); + + it('merges with an existing config file', async () => { + const existingContent = `[opik]\napi_key = old-key\nproject_name = old-project\n`; + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(existingContent); + + await saveToOpikConfigStep({ + projectName: 'new-project', + urlOverride: 'https://www.comet.com/opik/api', + apiKey: 'new-key', + }); + + expect(fs.promises.writeFile).toHaveBeenCalledOnce(); + const written = vi.mocked(fs.promises.writeFile).mock.calls[0][1] as string; + expect(written).toContain('project_name = new-project'); + expect(written).toContain('api_key = new-key'); + expect(written).not.toContain('old-project'); + }); + + it('does not throw when writeFile fails — logs a warning instead', async () => { + vi.mocked(fs.promises.writeFile).mockRejectedValue(new Error('EACCES')); + + await expect( + saveToOpikConfigStep({ + projectName: 'proj', + urlOverride: 'http://localhost/api', + }), + ).resolves.not.toThrow(); + }); +}); diff --git a/sdks/typescript/src/opik/configure/tsconfig.json b/sdks/typescript/src/opik/configure/tsconfig.json index b9f78272740..1ac6816a13e 100644 --- a/sdks/typescript/src/opik/configure/tsconfig.json +++ b/sdks/typescript/src/opik/configure/tsconfig.json @@ -16,6 +16,6 @@ "types": ["node"], "typeRoots": ["./node_modules/@types", "./types"] }, - "include": ["src", "bin.ts", "index.ts", "types"], + "include": ["src", "tests", "bin.ts", "index.ts", "types"], "exclude": ["node_modules", "dist"] } diff --git a/sdks/typescript/src/opik/configure/vitest.config.ts b/sdks/typescript/src/opik/configure/vitest.config.ts new file mode 100644 index 00000000000..80d928de794 --- /dev/null +++ b/sdks/typescript/src/opik/configure/vitest.config.ts @@ -0,0 +1,9 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + globals: true, + environment: 'node', + include: ['tests/**/*.test.ts'], + }, +}); \ No newline at end of file From 58bd588f970bfcaa6f335c8a3d2dd6774397297d Mon Sep 17 00:00:00 2001 From: Iaroslav Omelianenko Date: Wed, 8 Apr 2026 18:11:03 +0300 Subject: [PATCH 3/5] [OPIK-5696] Refactor configuration handling to use `ini` package and improve file path resolution - Replaced custom INI parsing/serialization logic with the `ini` package to simplify code and improve maintainability. - Added support for `OPIK_CONFIG_PATH` environment variable to allow custom configuration file paths. - Implemented fallback mechanism for invalid or non-existent `OPIK_CONFIG_PATH` directories. - Updated tests to reflect `ini` package integration and new path resolution logic. --- .../src/opik/configure/package.json | 3 +- .../src/steps/save-to-opik-config.ts | 84 ++------ .../tests/steps/save-to-opik-config.test.ts | 199 +++++++----------- 3 files changed, 94 insertions(+), 192 deletions(-) diff --git a/sdks/typescript/src/opik/configure/package.json b/sdks/typescript/src/opik/configure/package.json index 9f66fc8fa52..2a78c7750f7 100644 --- a/sdks/typescript/src/opik/configure/package.json +++ b/sdks/typescript/src/opik/configure/package.json @@ -37,10 +37,11 @@ "@clack/core": "^1.0.0", "@clack/prompts": "1.0.0", "@langchain/core": "^1.1.19", - "axios": "1.13.4", + "axios": "1.13.5", "chalk": "^5.6.2", "fast-glob": "^3.3.3", "glob": "13.0.1", + "ini": "^5.0.0", "inquirer": "^13.2.2", "jsonc-parser": "^3.3.1", "lodash": "^4.17.23", diff --git a/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts b/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts index 776c45ac6f2..a48d7cdcced 100644 --- a/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts +++ b/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts @@ -1,72 +1,30 @@ import chalk from 'chalk'; import * as fs from 'fs'; +import ini from 'ini'; import * as os from 'os'; import * as path from 'path'; import clack from '../utils/clack'; -const OPIK_CONFIG_FILE = path.join(os.homedir(), '.opik.config'); +const OPIK_CONFIG_FILE_DEFAULT = path.join(os.homedir(), '.opik.config'); -/** Keys we read/write in the [opik] section */ -interface OpikIniSection { - api_key?: string; - url_override?: string; - workspace?: string; - project_name?: string; +function expandPath(filePath: string): string { + return filePath.replace(/^~(?=$|\/|\\)/, os.homedir()); } -/** Parse the [opik] section from a raw INI string (no external deps). */ -export function parseOpikSection(content: string): OpikIniSection { - const result: OpikIniSection = {}; - let inOpikSection = false; - - for (const raw of content.split('\n')) { - const line = raw.trim(); - - if (line === '[opik]') { - inOpikSection = true; - continue; - } - - if (line.startsWith('[')) { - inOpikSection = false; - continue; - } - - if (!inOpikSection || !line || line.startsWith('#') || line.startsWith(';')) { - continue; - } - - const eqIdx = line.indexOf('='); - if (eqIdx === -1) continue; - - const key = line.slice(0, eqIdx).trim() as keyof OpikIniSection; - (result as Record)[key] = line.slice(eqIdx + 1).trim().replace(/^["']|["']$/g, ''); - } - - return result; -} - -/** Serialise an OpikIniSection back to an INI string with an [opik] section. */ -export function serializeOpikSection( - section: OpikIniSection, - existingContent: string, -): string { - const opikLines = ['[opik]']; - for (const [key, value] of Object.entries(section)) { - if (value !== undefined && value !== '') { - opikLines.push(`${key} = ${value}`); - } +function resolveConfigFilePath(): string { + if (!process.env.OPIK_CONFIG_PATH) { + return OPIK_CONFIG_FILE_DEFAULT; } - const opikBlock = opikLines.join('\n'); - // Replace existing [opik] block if present, otherwise append - const opikSectionRegex = /\[opik][^[]*/s; - if (opikSectionRegex.test(existingContent)) { - return existingContent.replace(opikSectionRegex, opikBlock + '\n'); + const customPath = expandPath(process.env.OPIK_CONFIG_PATH); + if (!fs.existsSync(path.dirname(customPath))) { + clack.log.warning( + `OPIK_CONFIG_PATH parent directory does not exist: ${chalk.bold.cyan(path.dirname(customPath))}. Falling back to ${chalk.bold.cyan(OPIK_CONFIG_FILE_DEFAULT)}.`, + ); + return OPIK_CONFIG_FILE_DEFAULT; } - const trimmed = existingContent.trimEnd(); - return trimmed ? `${trimmed}\n\n${opikBlock}\n` : `${opikBlock}\n`; + return customPath; } export interface SaveToOpikConfigOptions { @@ -86,14 +44,15 @@ export async function saveToOpikConfigStep( const { projectName, urlOverride, apiKey, workspace } = options; try { - let existingContent = ''; - if (fs.existsSync(OPIK_CONFIG_FILE)) { - existingContent = fs.readFileSync(OPIK_CONFIG_FILE, 'utf8'); + const configFilePath = resolveConfigFilePath(); + let parsed: Record = {}; + if (fs.existsSync(configFilePath)) { + parsed = ini.parse(fs.readFileSync(configFilePath, 'utf8')); } - const existing = parseOpikSection(existingContent); + const existing = (parsed['opik'] as Record | undefined) ?? {}; - const merged: OpikIniSection = { + parsed['opik'] = { ...existing, url_override: urlOverride, project_name: projectName, @@ -101,8 +60,7 @@ export async function saveToOpikConfigStep( ...(workspace ? { workspace } : {}), }; - const newContent = serializeOpikSection(merged, existingContent); - await fs.promises.writeFile(OPIK_CONFIG_FILE, newContent, { + await fs.promises.writeFile(configFilePath, ini.stringify(parsed), { encoding: 'utf8', flag: 'w', }); diff --git a/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts b/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts index d3a154b03f0..dad3242a427 100644 --- a/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts +++ b/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts @@ -1,10 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as fs from 'fs'; -import { - parseOpikSection, - serializeOpikSection, - saveToOpikConfigStep, -} from '../../src/steps'; +import ini from 'ini'; +import { saveToOpikConfigStep } from '../../src/steps'; // ESM requires module-level mocking for native node modules. vi.mock('fs', async () => { @@ -25,114 +22,6 @@ vi.mock('../../src/utils/clack', () => ({ default: { log: { success: vi.fn(), warning: vi.fn() } }, })); -// --------------------------------------------------------------------------- -// parseOpikSection -// --------------------------------------------------------------------------- - -describe('parseOpikSection', () => { - it('parses all keys from an [opik] section', () => { - const content = ` -[opik] -api_key = my-api-key -url_override = https://example.com/api -workspace = my-workspace -project_name = my-project -`.trim(); - - expect(parseOpikSection(content)).toEqual({ - api_key: 'my-api-key', - url_override: 'https://example.com/api', - workspace: 'my-workspace', - project_name: 'my-project', - }); - }); - - it('strips surrounding quotes from values', () => { - const content = `[opik]\napi_key = "quoted-key"\nproject_name = 'single-quoted'`; - const result = parseOpikSection(content); - expect(result.api_key).toBe('quoted-key'); - expect(result.project_name).toBe('single-quoted'); - }); - - it('returns empty object when [opik] section is absent', () => { - expect(parseOpikSection('')).toEqual({}); - expect(parseOpikSection('[other]\nfoo = bar')).toEqual({}); - }); - - it('stops reading at the next section header', () => { - const content = `[opik]\nproject_name = proj\n[other]\napi_key = should-not-appear`; - const result = parseOpikSection(content); - expect(result.project_name).toBe('proj'); - expect(result.api_key).toBeUndefined(); - }); - - it('ignores comment lines', () => { - const content = `[opik]\n# this is a comment\n; another comment\nproject_name = proj`; - expect(parseOpikSection(content).project_name).toBe('proj'); - }); - - it('handles keys without values gracefully (no = sign)', () => { - const content = `[opik]\nbadline\nproject_name = proj`; - expect(parseOpikSection(content).project_name).toBe('proj'); - }); -}); - -// --------------------------------------------------------------------------- -// serializeOpikSection -// --------------------------------------------------------------------------- - -describe('serializeOpikSection', () => { - it('creates a new [opik] block when file is empty', () => { - const result = serializeOpikSection( - { project_name: 'proj', url_override: 'http://localhost/api' }, - '', - ); - expect(result).toContain('[opik]'); - expect(result).toContain('project_name = proj'); - expect(result).toContain('url_override = http://localhost/api'); - }); - - it('replaces an existing [opik] block in-place', () => { - const existing = `[opik]\nproject_name = old\nurl_override = http://old/api\n`; - const result = serializeOpikSection( - { project_name: 'new', url_override: 'http://new/api' }, - existing, - ); - expect(result).toContain('project_name = new'); - expect(result).toContain('url_override = http://new/api'); - expect(result).not.toContain('project_name = old'); - }); - - it('preserves other sections when replacing [opik]', () => { - const existing = `[other]\nfoo = bar\n\n[opik]\nproject_name = old\n`; - const result = serializeOpikSection({ project_name: 'new' }, existing); - expect(result).toContain('[other]'); - expect(result).toContain('foo = bar'); - expect(result).toContain('project_name = new'); - expect(result).not.toContain('project_name = old'); - }); - - it('appends [opik] when other sections exist but no [opik]', () => { - const existing = `[other]\nfoo = bar\n`; - const result = serializeOpikSection({ project_name: 'proj' }, existing); - expect(result).toContain('[other]'); - expect(result).toContain('[opik]'); - expect(result).toContain('project_name = proj'); - }); - - it('omits keys whose value is empty or undefined', () => { - const result = serializeOpikSection( - { project_name: 'proj', api_key: '' }, - '', - ); - expect(result).not.toContain('api_key'); - }); -}); - -// --------------------------------------------------------------------------- -// saveToOpikConfigStep -// --------------------------------------------------------------------------- - describe('saveToOpikConfigStep', () => { beforeEach(() => { vi.mocked(fs.existsSync).mockReturnValue(false); @@ -144,6 +33,11 @@ describe('saveToOpikConfigStep', () => { vi.clearAllMocks(); }); + function getWrittenParsed(): Record> { + const raw = vi.mocked(fs.promises.writeFile).mock.calls[0][1] as string; + return ini.parse(raw) as Record>; + } + it('creates the config file when it does not exist', async () => { await saveToOpikConfigStep({ projectName: 'my-project', @@ -151,12 +45,11 @@ describe('saveToOpikConfigStep', () => { }); expect(fs.promises.writeFile).toHaveBeenCalledOnce(); - const written = vi.mocked(fs.promises.writeFile).mock.calls[0][1] as string; - expect(written).toContain('[opik]'); - expect(written).toContain('project_name = my-project'); - expect(written).toContain('url_override = http://localhost/api'); - expect(written).not.toContain('api_key'); - expect(written).not.toContain('workspace'); + const written = getWrittenParsed(); + expect(written.opik.project_name).toBe('my-project'); + expect(written.opik.url_override).toBe('http://localhost/api'); + expect(written.opik.api_key).toBeUndefined(); + expect(written.opik.workspace).toBeUndefined(); }); it('includes api_key and workspace for cloud deployments', async () => { @@ -168,13 +61,15 @@ describe('saveToOpikConfigStep', () => { }); expect(fs.promises.writeFile).toHaveBeenCalledOnce(); - const written = vi.mocked(fs.promises.writeFile).mock.calls[0][1] as string; - expect(written).toContain('api_key = secret-key'); - expect(written).toContain('workspace = my-workspace'); + const written = getWrittenParsed(); + expect(written.opik.api_key).toBe('secret-key'); + expect(written.opik.workspace).toBe('my-workspace'); }); it('merges with an existing config file', async () => { - const existingContent = `[opik]\napi_key = old-key\nproject_name = old-project\n`; + const existingContent = ini.stringify({ + opik: { api_key: 'old-key', project_name: 'old-project' }, + }); vi.mocked(fs.existsSync).mockReturnValue(true); vi.mocked(fs.readFileSync).mockReturnValue(existingContent); @@ -185,10 +80,58 @@ describe('saveToOpikConfigStep', () => { }); expect(fs.promises.writeFile).toHaveBeenCalledOnce(); - const written = vi.mocked(fs.promises.writeFile).mock.calls[0][1] as string; - expect(written).toContain('project_name = new-project'); - expect(written).toContain('api_key = new-key'); - expect(written).not.toContain('old-project'); + const written = getWrittenParsed(); + expect(written.opik.project_name).toBe('new-project'); + expect(written.opik.api_key).toBe('new-key'); + }); + + it('preserves other sections from an existing config file', async () => { + const existingContent = ini.stringify({ + other: { foo: 'bar' }, + opik: { project_name: 'old-project' }, + }); + vi.mocked(fs.existsSync).mockReturnValue(true); + vi.mocked(fs.readFileSync).mockReturnValue(existingContent); + + await saveToOpikConfigStep({ + projectName: 'new-project', + urlOverride: 'http://localhost/api', + }); + + const written = getWrittenParsed(); + expect(written.other.foo).toBe('bar'); + expect(written.opik.project_name).toBe('new-project'); + }); + + it('writes to OPIK_CONFIG_PATH when the env var points to a path with an existing parent directory', async () => { + const customPath = '/custom/path/.opik.config'; + process.env.OPIK_CONFIG_PATH = customPath; + vi.mocked(fs.existsSync).mockReturnValue(true); // parent dir exists + + await saveToOpikConfigStep({ + projectName: 'proj', + urlOverride: 'http://localhost/api', + }); + + expect(fs.promises.writeFile).toHaveBeenCalledOnce(); + expect(vi.mocked(fs.promises.writeFile).mock.calls[0][0]).toBe(customPath); + delete process.env.OPIK_CONFIG_PATH; + }); + + it('falls back to default path when OPIK_CONFIG_PATH parent directory does not exist', async () => { + process.env.OPIK_CONFIG_PATH = '/nonexistent/dir/.opik.config'; + vi.mocked(fs.existsSync).mockReturnValue(false); // parent dir missing + + await saveToOpikConfigStep({ + projectName: 'proj', + urlOverride: 'http://localhost/api', + }); + + expect(fs.promises.writeFile).toHaveBeenCalledOnce(); + const writtenPath = vi.mocked(fs.promises.writeFile).mock.calls[0][0] as string; + expect(writtenPath).not.toBe('/nonexistent/dir/.opik.config'); + expect(writtenPath).toMatch(/\.opik\.config$/); + delete process.env.OPIK_CONFIG_PATH; }); it('does not throw when writeFile fails — logs a warning instead', async () => { @@ -201,4 +144,4 @@ describe('saveToOpikConfigStep', () => { }), ).resolves.not.toThrow(); }); -}); +}); \ No newline at end of file From 8cf73f17c646b64c362d85314f29bf7b266f7269 Mon Sep 17 00:00:00 2001 From: Iaroslav Omelianenko Date: Wed, 8 Apr 2026 18:26:42 +0300 Subject: [PATCH 4/5] [OPIK-5696] Ensure parent directory creation for custom OPIK_CONFIG_PATH and update fallback handling - Added logic to create directories recursively when `OPIK_CONFIG_PATH` parent directory is missing. - Improved error handling and warnings for directory creation failures. - Enhanced tests to validate directory creation and fallback behaviors. --- .../src/steps/save-to-opik-config.ts | 22 +++++++++++------- .../tests/steps/save-to-opik-config.test.ts | 23 +++++++++++++++++-- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts b/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts index a48d7cdcced..704eb131408 100644 --- a/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts +++ b/sdks/typescript/src/opik/configure/src/steps/save-to-opik-config.ts @@ -17,11 +17,17 @@ function resolveConfigFilePath(): string { } const customPath = expandPath(process.env.OPIK_CONFIG_PATH); - if (!fs.existsSync(path.dirname(customPath))) { - clack.log.warning( - `OPIK_CONFIG_PATH parent directory does not exist: ${chalk.bold.cyan(path.dirname(customPath))}. Falling back to ${chalk.bold.cyan(OPIK_CONFIG_FILE_DEFAULT)}.`, - ); - return OPIK_CONFIG_FILE_DEFAULT; + const parentDir = path.dirname(customPath); + + if (!fs.existsSync(parentDir)) { + try { + fs.mkdirSync(parentDir, { recursive: true }); + } catch (error) { + clack.log.warning( + `OPIK_CONFIG_PATH parent directory ${chalk.bold.cyan(parentDir)} could not be created: ${error instanceof Error ? error.message : String(error)}. Falling back to ${chalk.bold.cyan(OPIK_CONFIG_FILE_DEFAULT)}.`, + ); + return OPIK_CONFIG_FILE_DEFAULT; + } } return customPath; @@ -42,9 +48,9 @@ export async function saveToOpikConfigStep( options: SaveToOpikConfigOptions, ): Promise { const { projectName, urlOverride, apiKey, workspace } = options; + const configFilePath = resolveConfigFilePath(); try { - const configFilePath = resolveConfigFilePath(); let parsed: Record = {}; if (fs.existsSync(configFilePath)) { parsed = ini.parse(fs.readFileSync(configFilePath, 'utf8')); @@ -66,11 +72,11 @@ export async function saveToOpikConfigStep( }); clack.log.success( - `Saved Opik configuration to ${chalk.bold.cyan('~/.opik.config')}`, + `Saved Opik configuration to ${chalk.bold.cyan(configFilePath)}`, ); } catch (error) { clack.log.warning( - `Failed to save configuration to ${chalk.bold.cyan('~/.opik.config')}: ${error instanceof Error ? error.message : String(error)}`, + `Failed to save configuration to ${chalk.bold.cyan(configFilePath)}: ${error instanceof Error ? error.message : String(error)}`, ); } } \ No newline at end of file diff --git a/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts b/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts index dad3242a427..7e33d84d99b 100644 --- a/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts +++ b/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts @@ -10,6 +10,7 @@ vi.mock('fs', async () => { ...actual, existsSync: vi.fn(), readFileSync: vi.fn(), + mkdirSync: vi.fn(), promises: { ...actual.promises, writeFile: vi.fn(), @@ -118,9 +119,27 @@ describe('saveToOpikConfigStep', () => { delete process.env.OPIK_CONFIG_PATH; }); - it('falls back to default path when OPIK_CONFIG_PATH parent directory does not exist', async () => { - process.env.OPIK_CONFIG_PATH = '/nonexistent/dir/.opik.config'; + it('creates parent directory and writes to OPIK_CONFIG_PATH when parent dir is missing', async () => { + const customPath = '/nonexistent/dir/.opik.config'; + process.env.OPIK_CONFIG_PATH = customPath; vi.mocked(fs.existsSync).mockReturnValue(false); // parent dir missing + vi.mocked(fs.mkdirSync).mockReturnValue(undefined); + + await saveToOpikConfigStep({ + projectName: 'proj', + urlOverride: 'http://localhost/api', + }); + + expect(fs.mkdirSync).toHaveBeenCalledWith('/nonexistent/dir', { recursive: true }); + expect(fs.promises.writeFile).toHaveBeenCalledOnce(); + expect(vi.mocked(fs.promises.writeFile).mock.calls[0][0]).toBe(customPath); + delete process.env.OPIK_CONFIG_PATH; + }); + + it('falls back to default path when OPIK_CONFIG_PATH parent dir cannot be created', async () => { + process.env.OPIK_CONFIG_PATH = '/nonexistent/dir/.opik.config'; + vi.mocked(fs.existsSync).mockReturnValue(false); + vi.mocked(fs.mkdirSync).mockImplementation(() => { throw new Error('EACCES'); }); await saveToOpikConfigStep({ projectName: 'proj', From 2467b7ceb813910e91c886cf90a5299145d608ee Mon Sep 17 00:00:00 2001 From: Iaroslav Omelianenko Date: Wed, 8 Apr 2026 18:52:26 +0300 Subject: [PATCH 5/5] [OPIK-5696] Add environment isolation for `OPIK_CONFIG_PATH` in tests - Introduced setup/teardown logic to ensure `OPIK_CONFIG_PATH` is isolated during tests. - Removed redundant `process.env.OPIK_CONFIG_PATH` cleanup from individual test cases. --- .../tests/steps/save-to-opik-config.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts b/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts index 7e33d84d99b..08704498777 100644 --- a/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts +++ b/sdks/typescript/src/opik/configure/tests/steps/save-to-opik-config.test.ts @@ -24,13 +24,22 @@ vi.mock('../../src/utils/clack', () => ({ })); describe('saveToOpikConfigStep', () => { + let originalConfigPath: string | undefined; + beforeEach(() => { + originalConfigPath = process.env.OPIK_CONFIG_PATH; + delete process.env.OPIK_CONFIG_PATH; vi.mocked(fs.existsSync).mockReturnValue(false); vi.mocked(fs.readFileSync).mockReturnValue(''); vi.mocked(fs.promises.writeFile).mockResolvedValue(undefined); }); afterEach(() => { + if (originalConfigPath !== undefined) { + process.env.OPIK_CONFIG_PATH = originalConfigPath; + } else { + delete process.env.OPIK_CONFIG_PATH; + } vi.clearAllMocks(); }); @@ -116,7 +125,6 @@ describe('saveToOpikConfigStep', () => { expect(fs.promises.writeFile).toHaveBeenCalledOnce(); expect(vi.mocked(fs.promises.writeFile).mock.calls[0][0]).toBe(customPath); - delete process.env.OPIK_CONFIG_PATH; }); it('creates parent directory and writes to OPIK_CONFIG_PATH when parent dir is missing', async () => { @@ -133,7 +141,6 @@ describe('saveToOpikConfigStep', () => { expect(fs.mkdirSync).toHaveBeenCalledWith('/nonexistent/dir', { recursive: true }); expect(fs.promises.writeFile).toHaveBeenCalledOnce(); expect(vi.mocked(fs.promises.writeFile).mock.calls[0][0]).toBe(customPath); - delete process.env.OPIK_CONFIG_PATH; }); it('falls back to default path when OPIK_CONFIG_PATH parent dir cannot be created', async () => { @@ -150,7 +157,6 @@ describe('saveToOpikConfigStep', () => { const writtenPath = vi.mocked(fs.promises.writeFile).mock.calls[0][0] as string; expect(writtenPath).not.toBe('/nonexistent/dir/.opik.config'); expect(writtenPath).toMatch(/\.opik\.config$/); - delete process.env.OPIK_CONFIG_PATH; }); it('does not throw when writeFile fails — logs a warning instead', async () => {