Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix missing globals #611

Merged
merged 7 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions docs/genaisrc/genaiscript.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 0 additions & 6 deletions genaisrc/genaiscript.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 27 additions & 3 deletions genaisrc/test-gen.genai.js → genaisrc/test-gen.genai.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,14 @@ generate a plan to test the source code in each file

## Step 2

For each generate test, implement the TypeScript source code in a test file with suffix ".genai.test.ts"
For each generated test, implement the TypeScript source code in a test file with suffix ".test.ts"
in the same folder as the source file.


- always organize tests using 'describe' blocks
- this is imporant, generate all the source code
- use "describe", "test", "beforeEach" from the "node:test" test runner framework

${fence('import test, { beforeEach, describe } from "node:test"', "js")}
${fence('import test, { beforeEach, describe } from "node:test"', { language: "js" })}

- use "assert" from node:assert/strict (default export)
- the test title should describe the tested function
Expand All @@ -40,4 +41,27 @@ ${fence('import test, { beforeEach, describe } from "node:test"', "js")}
- if you need to create files, place them under a "temp" folder
- use Partial<T> to declare a partial type of a type T
- do NOT generate negative test cases

## Step 3

Validate and fix test sources.

Use 'run_test' tool to execute the generated test code and fix the test code to make tests pass.

- this is important.
`

defTool(
"run_test",
"run test code with node:test",
{
filename: "full path to the test file",
source: "source of the test file",
},
async (args) => {
const { filename, source } = args
console.debug(`running test code ${filename}`)
await workspace.writeText(filename, source)
return host.exec(`node`, ["--import", "tsx", "--test", filename])
}
)
9 changes: 9 additions & 0 deletions packages/cli/src/nodehost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import { logVerbose, unique } from "../../core/src/util"
import { parseModelIdentifier } from "../../core/src/models"
import { createAzureToken } from "./azuretoken"
import { LanguageModel } from "../../core/src/chat"
import { errorMessage } from "../../core/src/error"

class NodeServerManager implements ServerManager {
async start(): Promise<void> {
Expand Down Expand Up @@ -297,6 +298,14 @@ export class NodeHost implements RuntimeHost {
if (stdout) trace?.detailsFenced(`📩 stdout`, stdout)
if (stderr) trace?.detailsFenced(`📩 stderr`, stderr)
return { stdout, stderr, exitCode, failed }
} catch (err) {
trace?.error("exec failed", err)
return {
stdout: "",
stderr: errorMessage(err),
exitCode: 1,
failed: true,
}
} finally {
trace?.endDetails()
}
Expand Down
26 changes: 0 additions & 26 deletions packages/core/src/crypto.genai.test.ts

This file was deleted.

36 changes: 36 additions & 0 deletions packages/core/src/crypto.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import assert from 'node:assert/strict';
import test, { beforeEach, describe } from 'node:test';
import { randomHex } from './crypto';

describe('randomHex function', () => {
test('should generate a hex string of the correct length', () => {
const size = 16;
const hexString = randomHex(size);
assert.strictEqual(hexString.length, size * 2);
});

test('should ensure randomness in generated hex strings', () => {
const size = 16;
const hexString1 = randomHex(size);
const hexString2 = randomHex(size);
assert.notStrictEqual(hexString1, hexString2);
});

test('should handle the smallest valid size correctly', () => {
const size = 1;
const hexString = randomHex(size);
assert.strictEqual(hexString.length, 2);
});

test('should handle a large size correctly', () => {
const size = 1024;
const hexString = randomHex(size);
assert.strictEqual(hexString.length, size * 2);
});

test('should return an empty string for size 0', () => {
const size = 0;
const hexString = randomHex(size);
assert.strictEqual(hexString, '');
});
});
2 changes: 1 addition & 1 deletion packages/core/src/crypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ export function randomHex(size: number) {
const bytes = new Uint8Array(size)
crypto.getRandomValues(bytes)
return toHex(bytes)
}
}
117 changes: 117 additions & 0 deletions packages/core/src/error.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
import { strict as assert } from 'node:assert';
import { describe, it as test } from 'node:test';
import {
serializeError,
errorMessage,
CancelError,
NotSupportedError,
RequestError,
isCancelError,
isRequestError
} from './error';

describe('Error Utilities', () => {
describe('serializeError function', () => {
test('should return undefined for null or undefined input', () => {
assert.strictEqual(serializeError(null), undefined);
assert.strictEqual(serializeError(undefined), undefined);
});

test('should serialize an Error instance', () => {
const error = new Error('Test error');
const serialized = serializeError(error);
assert.strictEqual(serialized.message, 'Test error');
assert.ok('stack' in serialized);
});

test('should return the object as is for SerializedError input', () => {
const serializedError = { message: 'Serialized error', stack: 'stack trace' };
const serialized = serializeError(serializedError);
assert.deepStrictEqual(serialized, serializedError);
});

test('should return an object with message property for string input', () => {
const message = 'Test message';
const serialized = serializeError(message);
assert.strictEqual(serialized.message, message);
});

test('should return an object with message property for number input', () => {
const number = 42;
const serialized = serializeError(number);
assert.strictEqual(serialized.message, '42');
});
});

describe('errorMessage function', () => {
test('should return undefined for null or undefined input', () => {
assert.strictEqual(errorMessage(null), undefined);
assert.strictEqual(errorMessage(undefined), undefined);
});

test('should return the error message if available', () => {
const error = new Error('Test error message');
assert.strictEqual(errorMessage(error), 'Test error message');
});

test('should return default value if no message or name on error', () => {
const error = {}; // Empty error-like object
assert.strictEqual(errorMessage(error), 'error');
});
});

describe('CancelError class', () => {
test('should have a name property set to "CancelError"', () => {
const error = new CancelError('Cancellation happened');
assert.strictEqual(error.name, CancelError.NAME);
});
});

describe('NotSupportedError class', () => {
test('should have a name property set to "NotSupportedError"', () => {
const error = new NotSupportedError('Not supported');
assert.strictEqual(error.name, NotSupportedError.NAME);
});
});

describe('RequestError class', () => {
test('should set instance properties correctly', () => {
const status = 404;
const statusText = 'Not Found';
const body = { message: 'Resource not found' };
const bodyText = 'Error body text';
const retryAfter = 120;
const error = new RequestError(status, statusText, body, bodyText, retryAfter);
assert.strictEqual(error.status, status);
assert.strictEqual(error.statusText, statusText);
assert.deepStrictEqual(error.body, body);
assert.strictEqual(error.bodyText, bodyText);
assert.strictEqual(error.retryAfter, retryAfter);
});
});

describe('isCancelError function', () => {
test('should return true for CancelError instances', () => {
const error = new CancelError('Cancellation');
assert.ok(isCancelError(error));
});

test('should return true for AbortError', () => {
const error = new Error('Abort');
error.name = 'AbortError';
assert.ok(isCancelError(error));
});
});

describe('isRequestError function', () => {
test('should return true for RequestError instances with matching statusCode and code', () => {
const error = new RequestError(400, 'Bad Request', { code: 'BadRequest' });
assert.ok(isRequestError(error, 400, 'BadRequest'));
});

test('should return true for RequestError instances with undefined statusCode or code', () => {
const error = new RequestError(400, 'Bad Request', { code: 'BadRequest' });
assert.ok(isRequestError(error));
});
});
});
6 changes: 0 additions & 6 deletions packages/core/src/genaisrc/genaiscript.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions packages/core/src/importprompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,23 @@ export async function importPrompt(
if (!filename) throw new Error("filename is required")
const { trace } = options || {}

const leakables = [
"host",
"workspace",
"path",
"parsers",
"env",
"retrieval",
"YAML",
"INI",
"CSV",
"XML",
"JSONL",
"AICI",
"fetchText",
"cancel",
]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The array 'leakables' seems to be hard-coded. Consider making it a constant or configurable if these values are used elsewhere or could change.

generated by pr-review-commit hard_coded


const oldGlb: any = {}
const glb: any = resolveGlobal()
let unregister: () => void = undefined
Expand Down Expand Up @@ -68,6 +85,7 @@ export async function importPrompt(
} finally {
// restore global context
for (const field of Object.keys(oldGlb)) {
if (leakables.includes(field)) continue
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The array leakables contains global variables that could potentially be leaked into the global scope. This could lead to unexpected behavior.

generated by pr-review-commit leakable_globals

const v = oldGlb[field]
if (v === undefined) delete glb[field]
else glb[field] = oldGlb[field]
Expand Down
6 changes: 0 additions & 6 deletions packages/core/src/types/prompt_type.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,6 @@ declare var retrieval: Retrieval
*/
declare var workspace: WorkspaceFileSystem
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'fs' variable is deprecated. Please make sure to replace all its usages with 'workspace'.

generated by pr-review-commit deprecated_code

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The global variable fs has been deprecated and removed, but it is still referenced in the code. Please replace it with the recommended workspace variable.

generated by pr-review-commit deprecated_global


/**
* Access to the workspace file system.
* @deprecated Use `workspace` instead.
*/
declare var fs: WorkspaceFileSystem

/**
* YAML parsing and stringifying functions.
*/
Expand Down
14 changes: 0 additions & 14 deletions packages/sample/genaisrc/exec.genai.js

This file was deleted.

20 changes: 20 additions & 0 deletions packages/sample/genaisrc/exec.genai.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
script({
model: "openai:gpt-3.5-turbo",
tests: {
keywords: ["Python", "3."],
},
})
const version = await host.exec("python", ["--version"])
if (!/^python \d/i.test(version.stdout))
throw new Error("python --version failed")
defTool(
"python_version",
"determines the version of python on this system",
{},
async (_) => {
console.debug(`determining python version`)
const version = await host.exec("python", ["--version"])
return version.stdout?.replace(/^python\s/i, "") || "?"
}
)
$`What is the version of python on this machine?`
6 changes: 0 additions & 6 deletions packages/sample/genaisrc/genaiscript.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading