From 278e4cb4c27b470858ce8c68f42434ef6d928a5a Mon Sep 17 00:00:00 2001 From: Takanori Matsumoto Date: Mon, 6 May 2024 00:46:15 +0900 Subject: [PATCH] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(config.ts):=20Add?= =?UTF-8?q?ition=20of=20UnitTest=20environment=20and=20unittest=20for=20co?= =?UTF-8?q?mmands/config.ts#getConfig=20(#330)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(jest.config.ts): update jest preset for TS ESM support and ignore patterns feat(package.json): add test:unit script with NODE_OPTIONS for ESM refactor(src/commands/config.ts): improve dotenv usage with dynamic paths feat(src/commands/config.ts): allow custom config and env paths in getConfig refactor(src/commands/config.ts): streamline environment variable access feat(test/unit): add unit tests for config handling and utility functions - Implement unit tests for `getConfig` function to ensure correct behavior in various scenarios including default values, global config, and local env file precedence. - Add utility function `prepareFile` for creating temporary files during tests, facilitating testing of file-based configurations. * feat(e2e.yml): add unit-test job to GitHub Actions for running unit tests on pull requests * ci(test.yml): add GitHub Actions workflow for unit and e2e tests on pull requests * refactor(config.ts): streamline environment variable access using process.env directly test(config.test.ts): add setup and teardown for environment variables in tests to ensure test isolation * feat(package.json): add `test:all` script to run all tests in Docker refactor(package.json): consolidate Docker build steps into `test:docker-build` script for DRY principle fix(package.json): ensure `test:unit:docker` and `test:e2e:docker` scripts use the same Docker image and remove container after run chore(test/Dockerfile): remove default CMD to allow dynamic test script execution in Docker * refactor(config.test.ts): anonymize API keys in tests for better security practices * feat(config.test.ts): add tests for OCO_ANTHROPIC_API_KEY configuration * refactor(config.ts): streamline path imports and remove unused DotenvParseOutput - Simplify path module imports by removing default import and using named imports for `pathJoin` and `pathResolve`. - Remove unused `DotenvParseOutput` import to clean up the code. * refactor(config.test.ts): simplify API key mock values for clarity in tests * test(config.test.ts): remove tests for default config values and redundant cases - Removed tests that checked for default config values when no config or env files are present, as these scenarios are now handled differently. - Eliminated tests for empty global config and local env files to streamline testing focus on actual config loading logic. - Removed test for prioritizing local env over global config due to changes in config loading strategy, simplifying the configuration management. --- .github/workflows/{e2e.yml => test.yml} | 17 +++- jest.config.ts | 4 +- package.json | 6 +- src/commands/config.ts | 18 ++-- test/Dockerfile | 2 - test/unit/config.test.ts | 105 ++++++++++++++++++++++++ test/unit/utils.ts | 29 +++++++ 7 files changed, 170 insertions(+), 11 deletions(-) rename .github/workflows/{e2e.yml => test.yml} (63%) create mode 100644 test/unit/config.test.ts create mode 100644 test/unit/utils.ts diff --git a/.github/workflows/e2e.yml b/.github/workflows/test.yml similarity index 63% rename from .github/workflows/e2e.yml rename to .github/workflows/test.yml index f4f092d..b1e49e3 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/test.yml @@ -1,8 +1,23 @@ -name: E2E Testing +name: Testing on: [pull_request] jobs: + unit-test: + runs-on: ubuntu-latest + strategy: + matrix: + node-version: [20.x] + steps: + - uses: actions/checkout@v2 + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v2 + with: + node-version: ${{ matrix.node-version }} + - name: Install dependencies + run: npm install + - name: Run Unit Tests + run: npm run test:unit e2e-test: runs-on: ubuntu-latest strategy: diff --git a/jest.config.ts b/jest.config.ts index 2130dc1..6bca393 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -11,15 +11,17 @@ const config: Config = { "node_modules", "src", ], - preset: 'ts-jest', + preset: 'ts-jest/presets/js-with-ts-esm', setupFilesAfterEnv: ['/test/jest-setup.ts'], testEnvironment: "node", testRegex: [ '.*\\.test\\.ts$', ], + transformIgnorePatterns: ['node_modules/(?!cli-testing-library)'], transform: { '^.+\\.(ts|tsx)$': ['ts-jest', { diagnostics: false, + useESM: true }], } }; diff --git a/package.json b/package.json index 56e6a65..6764a61 100644 --- a/package.json +++ b/package.json @@ -48,8 +48,12 @@ "deploy": "npm version patch && npm run build:push && git push --tags && npm publish --tag latest", "lint": "eslint src --ext ts && tsc --noEmit", "format": "prettier --write src", + "test:all": "npm run test:unit:docker && npm run test:e2e:docker", + "test:docker-build": "docker build -t oco-test -f test/Dockerfile .", + "test:unit": "NODE_OPTIONS=--experimental-vm-modules jest test/unit", + "test:unit:docker": "npm run test:docker-build && DOCKER_CONTENT_TRUST=0 docker run --rm oco-test npm run test:unit", "test:e2e": "jest test/e2e", - "test:e2e:docker": "docker build -t oco-e2e -f test/Dockerfile . && DOCKER_CONTENT_TRUST=0 docker run oco-e2e" + "test:e2e:docker": "npm run test:docker-build && DOCKER_CONTENT_TRUST=0 docker run --rm oco-test npm run test:e2e" }, "devDependencies": { "@commitlint/types": "^17.4.4", diff --git a/src/commands/config.ts b/src/commands/config.ts index e038312..d877db3 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -4,15 +4,13 @@ import * as dotenv from 'dotenv'; import { existsSync, readFileSync, writeFileSync } from 'fs'; import { parse as iniParse, stringify as iniStringify } from 'ini'; import { homedir } from 'os'; -import { join as pathJoin } from 'path'; +import { join as pathJoin, resolve as pathResolve } from 'path'; import { intro, outro } from '@clack/prompts'; import { COMMANDS } from '../CommandsEnum'; import { getI18nLocal } from '../i18n'; -dotenv.config(); - export enum CONFIG_KEYS { OCO_OPENAI_API_KEY = 'OCO_OPENAI_API_KEY', OCO_ANTHROPIC_API_KEY = 'OCO_ANTHROPIC_API_KEY', @@ -248,9 +246,17 @@ export type ConfigType = { [key in CONFIG_KEYS]?: any; }; -const configPath = pathJoin(homedir(), '.opencommit'); +const defaultConfigPath = pathJoin(homedir(), '.opencommit'); +const defaultEnvPath = pathResolve(process.cwd(), '.env'); -export const getConfig = (): ConfigType | null => { +export const getConfig = ({ + configPath = defaultConfigPath, + envPath = defaultEnvPath +}: { + configPath?: string + envPath?: string +} = {}): ConfigType | null => { + dotenv.config({ path: envPath }); const configFromEnv = { OCO_OPENAI_API_KEY: process.env.OCO_OPENAI_API_KEY, OCO_ANTHROPIC_API_KEY: process.env.OCO_ANTHROPIC_API_KEY, @@ -306,7 +312,7 @@ export const getConfig = (): ConfigType | null => { return config; }; -export const setConfig = (keyValues: [key: string, value: string][]) => { +export const setConfig = (keyValues: [key: string, value: string][], configPath: string = defaultConfigPath) => { const config = getConfig() || {}; for (const [configKey, configValue] of keyValues) { diff --git a/test/Dockerfile b/test/Dockerfile index 80c8f3c..5b77917 100644 --- a/test/Dockerfile +++ b/test/Dockerfile @@ -17,5 +17,3 @@ RUN ls -la RUN npm install RUN npm run build - -CMD ["npm", "run", "test:e2e"] diff --git a/test/unit/config.test.ts b/test/unit/config.test.ts new file mode 100644 index 0000000..4c596ae --- /dev/null +++ b/test/unit/config.test.ts @@ -0,0 +1,105 @@ +import { getConfig } from '../../src/commands/config'; +import { prepareFile } from './utils'; + +describe('getConfig', () => { + const originalEnv = { ...process.env }; + function resetEnv(env: NodeJS.ProcessEnv) { + Object.keys(process.env).forEach((key) => { + if (!(key in env)) { + delete process.env[key]; + } else { + process.env[key] = env[key]; + } + }); + } + + beforeEach(() => { + resetEnv(originalEnv); + }); + + afterAll(() => { + resetEnv(originalEnv); + }); + + it('return config values from the global config file', async () => { + const configFile = await prepareFile( + '.opencommit', + ` +OCO_OPENAI_API_KEY="sk-key" +OCO_ANTHROPIC_API_KEY="secret-key" +OCO_TOKENS_MAX_INPUT="8192" +OCO_TOKENS_MAX_OUTPUT="1000" +OCO_OPENAI_BASE_PATH="/openai/api" +OCO_DESCRIPTION="true" +OCO_EMOJI="true" +OCO_MODEL="gpt-4" +OCO_LANGUAGE="de" +OCO_MESSAGE_TEMPLATE_PLACEHOLDER="$m" +OCO_PROMPT_MODULE="@commitlint" +OCO_AI_PROVIDER="ollama" +OCO_GITPUSH="false" +OCO_ONE_LINE_COMMIT="true" +` + ); + const config = getConfig({ configPath: configFile.filePath, envPath: '' }); + + expect(config).not.toEqual(null); + expect(config!['OCO_OPENAI_API_KEY']).toEqual('sk-key'); + expect(config!['OCO_ANTHROPIC_API_KEY']).toEqual('secret-key'); + expect(config!['OCO_TOKENS_MAX_INPUT']).toEqual(8192); + expect(config!['OCO_TOKENS_MAX_OUTPUT']).toEqual(1000); + expect(config!['OCO_OPENAI_BASE_PATH']).toEqual('/openai/api'); + expect(config!['OCO_DESCRIPTION']).toEqual(true); + expect(config!['OCO_EMOJI']).toEqual(true); + expect(config!['OCO_MODEL']).toEqual('gpt-4'); + expect(config!['OCO_LANGUAGE']).toEqual('de'); + expect(config!['OCO_MESSAGE_TEMPLATE_PLACEHOLDER']).toEqual('$m'); + expect(config!['OCO_PROMPT_MODULE']).toEqual('@commitlint'); + expect(config!['OCO_AI_PROVIDER']).toEqual('ollama'); + expect(config!['OCO_GITPUSH']).toEqual(false); + expect(config!['OCO_ONE_LINE_COMMIT']).toEqual(true); + + await configFile.cleanup(); + }); + + it('return config values from the local env file', async () => { + const envFile = await prepareFile( + '.env', + ` +OCO_OPENAI_API_KEY="sk-key" +OCO_ANTHROPIC_API_KEY="secret-key" +OCO_TOKENS_MAX_INPUT="8192" +OCO_TOKENS_MAX_OUTPUT="1000" +OCO_OPENAI_BASE_PATH="/openai/api" +OCO_DESCRIPTION="true" +OCO_EMOJI="true" +OCO_MODEL="gpt-4" +OCO_LANGUAGE="de" +OCO_MESSAGE_TEMPLATE_PLACEHOLDER="$m" +OCO_PROMPT_MODULE="@commitlint" +OCO_AI_PROVIDER="ollama" +OCO_GITPUSH="false" +OCO_ONE_LINE_COMMIT="true" + ` + ); + const config = getConfig({ configPath: '', envPath: envFile.filePath }); + + expect(config).not.toEqual(null); + expect(config!['OCO_OPENAI_API_KEY']).toEqual('sk-key'); + expect(config!['OCO_ANTHROPIC_API_KEY']).toEqual('secret-key'); + expect(config!['OCO_TOKENS_MAX_INPUT']).toEqual(8192); + expect(config!['OCO_TOKENS_MAX_OUTPUT']).toEqual(1000); + expect(config!['OCO_OPENAI_BASE_PATH']).toEqual('/openai/api'); + expect(config!['OCO_DESCRIPTION']).toEqual(true); + expect(config!['OCO_EMOJI']).toEqual(true); + expect(config!['OCO_MODEL']).toEqual('gpt-4'); + expect(config!['OCO_LANGUAGE']).toEqual('de'); + expect(config!['OCO_MESSAGE_TEMPLATE_PLACEHOLDER']).toEqual('$m'); + expect(config!['OCO_PROMPT_MODULE']).toEqual('@commitlint'); + expect(config!['OCO_AI_PROVIDER']).toEqual('ollama'); + expect(config!['OCO_GITPUSH']).toEqual(false); + expect(config!['OCO_ONE_LINE_COMMIT']).toEqual(true); + + await envFile.cleanup(); + }); +}); diff --git a/test/unit/utils.ts b/test/unit/utils.ts new file mode 100644 index 0000000..1443344 --- /dev/null +++ b/test/unit/utils.ts @@ -0,0 +1,29 @@ +import path from 'path'; +import { mkdtemp, rm, writeFile } from 'fs'; +import { promisify } from 'util'; +import { tmpdir } from 'os'; +const fsMakeTempDir = promisify(mkdtemp); +const fsRemove = promisify(rm); +const fsWriteFile = promisify(writeFile); + +/** + * Prepare tmp file for the test + */ +export async function prepareFile( + fileName: string, + content: string +): Promise<{ + filePath: string; + cleanup: () => Promise; +}> { + const tempDir = await fsMakeTempDir(path.join(tmpdir(), 'opencommit-test-')); + const filePath = path.resolve(tempDir, fileName); + await fsWriteFile(filePath, content); + const cleanup = async () => { + return fsRemove(tempDir, { recursive: true }); + }; + return { + filePath, + cleanup + }; +}