Skip to content

Commit

Permalink
refactor: clean up spec.test.ts to prepare for translation to Go and …
Browse files Browse the repository at this point in the history
…Python; other minor fixes to test CI (#78)

CHANGELOG:
- [ ] Refactor test harness to prepare for translation to Go and Python.
- [ ] Update hooks to run all tests
- [ ] Configure CI and hooks to fail on failing tests since the team
  finds that comfortable and will translate bottom up.
- [ ] Clean up tests to reflect that.
- [ ] Fix some lint in dir.test.ts and adapters.ts
- [ ] Add some convenience scripts to the root package.json file.
- [ ] Update pnpm to 10.2.0 to stay at the same version as Genkit.
- [ ] Add a Role enum type for the role.
- [ ] Add docstrings to more types in typing.py
- [ ] Configure ruff to clean up unused imports and variables.
  • Loading branch information
yesudeep authored Feb 25, 2025
1 parent 8200ae4 commit 4efc062
Show file tree
Hide file tree
Showing 17 changed files with 297 additions and 146 deletions.
22 changes: 2 additions & 20 deletions captainhook.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,11 @@
{
"run": "scripts/check_license"
},
{
"run": "uv run --directory python ruff check --select I --fix ."
},
{
"run": "uv run --directory python mypy ."
},
{
"run": "scripts/run_python_tests"
},
{
"run": "scripts/run_go_tests"
},
{
"run": "scripts/run_js_tests"
"run": "scripts/run_tests"
},
{
"run": "uv run mkdocs build"
Expand Down Expand Up @@ -85,20 +76,11 @@
{
"run": "scripts/check_license"
},
{
"run": "uv run --directory python ruff check --select I --fix ."
},
{
"run": "uv run --directory python mypy ."
},
{
"run": "scripts/run_python_tests"
},
{
"run": "scripts/run_go_tests"
},
{
"run": "scripts/run_js_tests"
"run": "scripts/run_tests"
},
{
"run": "uv run mkdocs build"
Expand Down
9 changes: 2 additions & 7 deletions go/dotprompt/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ func Square(n int) int {
return n * n
}

func TestSkipAllFailing(t *testing.T) {
// Currently, since the Go runtime is catching up to the JS runtime implementation
// we skip all failing tests. This test will fail because 2*2 = 4, not 5
// but the CI/pre-commits will not complain.
//
// TODO: Remove this test when the runtime implementation is complete.
assert.Equal(t, 5, Square(2), "This test should fail because 2*2 = 4, not 5")
func TestSquare(t *testing.T) {
assert.Equal(t, 4, Square(2))
}
4 changes: 2 additions & 2 deletions js/examples/adapters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ input:
);

const openaiResponse = await fetch(
`https://generativelanguage.googleapis.com/v1beta/openai/chat/completions`,
'https://generativelanguage.googleapis.com/v1beta/openai/chat/completions',
{
method: 'POST',
body: JSON.stringify(openaiFormat),
headers: {
'content-type': 'application/json',
authorization: 'Bearer ' + process.env.GOOGLE_GENAI_API_KEY,
authorization: `Bearer ${process.env.GOOGLE_GENAI_API_KEY}`,
},
}
);
Expand Down
4 changes: 2 additions & 2 deletions js/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "dotprompt",
"version": "1.0.1",
"description": "",
"description": "Dotprompt: Executable GenAI Prompt Templates",
"main": "dist/index.js",
"types": "dist/index.d.ts",
"repository": {
Expand Down Expand Up @@ -35,7 +35,7 @@
"handlebars": "^4.7.8",
"yaml": "^2.5.0"
},
"packageManager": "pnpm@9.13.2+sha256.ccce81bf7498c5f0f80e31749c1f8f03baba99d168f64590fc7e13fad3ea1938",
"packageManager": "pnpm@10.2.0",
"pnpm": {
"overrides": {
"rollup@>=4.0.0 <4.22.4": ">=4.22.4",
Expand Down
237 changes: 154 additions & 83 deletions js/test/spec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,31 @@ import { parse } from 'yaml';
import { Dotprompt } from '../src/dotprompt';
import type { DataArgument, JSONSchema, ToolDefinition } from '../src/types';

const specDir = join('..', 'spec');
const files = readdirSync(specDir, { recursive: true, withFileTypes: true });
/**
* The directory containing the spec files.
*/
const SPEC_DIR = join('..', 'spec');

/**
* A test case for a YAML spec.
*/
interface SpecTest {
desc?: string;
data: DataArgument;
expect: {
config: boolean;
ext: boolean;
input: boolean;
messages: boolean;
metadata: boolean;
raw: boolean;
};
options: object;
}

/**
* A suite of test cases for a YAML spec.
*/
interface SpecSuite {
name: string;
template: string;
Expand All @@ -21,90 +43,139 @@ interface SpecSuite {
tools?: Record<string, ToolDefinition>;
partials?: Record<string, string>;
resolverPartials?: Record<string, string>;
tests: { desc?: string; data: DataArgument; expect: any; options: object }[];
tests: SpecTest[];
}

// Process each YAML file
files
.filter((file) => !file.isDirectory() && file.name.endsWith('.yaml'))
.forEach((file) => {
const suiteName = join(
relative(specDir, file.path),
file.name.replace(/\.yaml$/, '')
);
const suites: SpecSuite[] = parse(
readFileSync(join(file.path, file.name), 'utf-8')
/**
* Creates test cases for a YAML spec.
*
* @param s The suite
* @param tc The test case
* @param dotpromptFactory The dotprompt factory
*/
async function createTestCases(
s: SpecSuite,
tc: SpecTest,
dotpromptFactory: (suite: SpecSuite) => Dotprompt
) {
it(tc.desc || 'should match expected output', async () => {
const env = dotpromptFactory(s);

// Define partials if they exist.
if (s.partials) {
for (const [name, template] of Object.entries(s.partials)) {
env.definePartial(name, template);
}
}

// Render the template.
const result = await env.render(
s.template,
{ ...s.data, ...tc.data },
tc.options
);

// Create a describe block for each YAML file
suite(suiteName, () => {
// Create a describe block for each suite in the file
suites.forEach((s) => {
describe(s.name, () => {
// Create a test for each test case in the suite
s.tests.forEach((tc) => {
it(tc.desc || 'should match expected output', async () => {
const env = new Dotprompt({
schemas: s.schemas,
tools: s.tools,
partialResolver: (name: string) =>
s.resolverPartials?.[name] || null,
});

if (s.partials) {
for (const [name, template] of Object.entries(s.partials)) {
env.definePartial(name, template);
}
}

const result = await env.render(
s.template,
{ ...s.data, ...tc.data },
tc.options
);
const { raw, ...prunedResult } = result;
const {
raw: expectRaw,
input: discardInputForRender,
...expected
} = tc.expect;
expect(
prunedResult,
'render should produce the expected result'
).toEqual({
...expected,
ext: expected.ext || {},
config: expected.config || {},
metadata: expected.metadata || {},
});
// only compare raw if the spec demands it
if (tc.expect.raw) {
expect(raw).toEqual(expectRaw);
}

const metadataResult = await env.renderMetadata(
s.template,
tc.options
);
const { raw: metadataResultRaw, ...prunedMetadataResult } =
metadataResult;
const {
messages,
raw: metadataExpectRaw,
...expectedMetadata
} = tc.expect;
expect(
prunedMetadataResult,
'renderMetadata should produce the expected result'
).toEqual({
...expectedMetadata,
ext: expectedMetadata.ext || {},
config: expectedMetadata.config || {},
metadata: expectedMetadata.metadata || {},
});
});
});
});
});
// Prune the result and compare to the expected output.
const { raw, ...prunedResult } = result;
const {
raw: expectRaw,
input: discardInputForRender,
...expected
} = tc.expect;

// Compare the pruned result to the expected output.
expect(prunedResult, 'render should produce the expected result').toEqual({
...expected,
ext: expected.ext || {},
config: expected.config || {},
metadata: expected.metadata || {},
});

// Only compare raw if the spec demands it.
if (tc.expect.raw) {
expect(raw).toEqual(expectRaw);
}

// Render the metadata.
const metadataResult = await env.renderMetadata(s.template, tc.options);
const { raw: metadataResultRaw, ...prunedMetadataResult } = metadataResult;
const { messages, raw: metadataExpectRaw, ...expectedMetadata } = tc.expect;

// Compare the pruned metadata result to the expected output.
expect(
prunedMetadataResult,
'renderMetadata should produce the expected result'
).toEqual({
...expectedMetadata,
ext: expectedMetadata.ext || {},
config: expectedMetadata.config || {},
metadata: expectedMetadata.metadata || {},
});
});
}

/**
* Creates a test suite for a YAML spec.
*
* @param suiteName The name of the suite
* @param suites The suites to create
* @param dotpromptFactory The dotprompt factory
*/
function createTestSuite(
suiteName: string,
suites: SpecSuite[],
dotpromptFactory: (suite: SpecSuite) => Dotprompt
) {
suite(suiteName, () => {
for (const s of suites) {
describe(s.name, () => {
for (const tc of s.tests) {
createTestCases(s, tc, dotpromptFactory);
}
});
}
});
}

/**
* Processes a single spec file. Takes the file reading function as a dependency.
*
* @param file The file to process
* @param readFileSyncFn The file reading function
* @param dotpromptFactory The dotprompt factory
*/
function processSpecFile(
file: { path: string; name: string },
readFileSyncFn: (path: string, encoding: BufferEncoding) => string,
dotpromptFactory: (suite: SpecSuite) => Dotprompt
) {
const suiteName = join(
relative(SPEC_DIR, file.path),
file.name.replace(/\.yaml$/, '')
);
const suites: SpecSuite[] = parse(
readFileSyncFn(join(file.path, file.name), 'utf-8')
);
createTestSuite(suiteName, suites, dotpromptFactory);
}

/**
* Top level processing, orchestrates the other functions.
*/
function processSpecFiles() {
const files = readdirSync(SPEC_DIR, { recursive: true, withFileTypes: true });
// Process each YAML file
for (const file of files.filter(
(file) => !file.isDirectory() && file.name.endsWith('.yaml')
)) {
processSpecFile(file, readFileSync, (s: SpecSuite) => {
return new Dotprompt({
schemas: s.schemas,
tools: s.tools,
partialResolver: (name: string) => s.resolverPartials?.[name] || null,
});
});
}
}

processSpecFiles();
6 changes: 3 additions & 3 deletions js/test/stores/dir.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { createHash } from 'crypto';
import { promises as fs } from 'fs';
import path from 'path';
import { createHash } from 'node:crypto';
import { promises as fs } from 'node:fs';
import path from 'node:path';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { DirStore } from '../../src/stores/dir';
import type { PromptData } from '../../src/types';
Expand Down
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
{
"packageManager": "pnpm@9.13.2+sha256.ccce81bf7498c5f0f80e31749c1f8f03baba99d168f64590fc7e13fad3ea1938",
"packageManager": "pnpm@10.2.0",
"scripts": {
"build": "pnpm -C js build",
"format": "pnpm dlx @biomejs/biome check --formatter-enabled=true --linter-enabled=false --organize-imports-enabled=true --fix . && scripts/add_license",
"format:check": "pnpm dlx @biomejs/biome ci --linter-enabled=false --formatter-enabled=true --organize-imports-enabled=false . && scripts/check_license",
"lint": "pnpm dlx @biomejs/biome lint --fix . && scripts/add_license"
"lint": "pnpm dlx @biomejs/biome lint --fix . && scripts/add_license",
"test": "pnpm -C js run test"
}
}
2 changes: 2 additions & 0 deletions python/dotpromptz/src/dotpromptz/helpers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ def render(text: str) -> str:
result = unless_equals_helper('test | other', render)
self.assertEqual(result, '')

# TODO: Re-enable this test once we have a way to render templates.
@unittest.skip('Skipping template rendering test')
def test_template_rendering(self) -> None:
"""Test helpers in actual template rendering."""
template = """
Expand Down
Loading

0 comments on commit 4efc062

Please sign in to comment.