-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
partners[minor]: Add standard chat model tests to partner packages #5660
Changes from all commits
4820bf9
400b04b
efb7f3f
1510d62
74d2fe9
f694aad
f9b4d85
5a8767e
bdb4d55
de2259c
8991340
183816c
b96008e
3f4a3b3
ded3663
18c748e
20d16d2
cc4f4e7
63de31e
c323fab
50971e2
01573ab
f978653
ca50201
74da454
ee55ad9
31aa8f8
d607d2b
4c0789b
e66d30e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,10 +16,6 @@ | |
"scripts": { | ||
"build": "yarn turbo:command build:internal --filter=@langchain/cloudflare", | ||
"build:internal": "yarn lc-build:v2 --create-entrypoints --pre --tree-shaking", | ||
"build:esm": "NODE_OPTIONS=--max-old-space-size=4096 tsc --outDir dist/ && rm -rf dist/tests dist/**/tests", | ||
"build:cjs": "NODE_OPTIONS=--max-old-space-size=4096 tsc --outDir dist-cjs/ -p tsconfig.cjs.json && yarn move-cjs-to-dist && rm -rf dist-cjs", | ||
"build:watch": "yarn create-entrypoints && tsc --outDir dist/ --watch", | ||
"build:scripts": "yarn create-entrypoints && yarn check-tree-shaking", | ||
"lint:eslint": "NODE_OPTIONS=--max-old-space-size=4096 eslint --cache --ext .ts,.js src/", | ||
"lint:dpdm": "dpdm --exit-code circular:1 --no-warning --no-tree src/*.ts src/**/*.ts", | ||
"lint": "yarn lint:eslint && yarn lint:dpdm", | ||
|
@@ -30,11 +26,11 @@ | |
"test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch --testPathIgnorePatterns=\\.int\\.test.ts", | ||
"test:single": "NODE_OPTIONS=--experimental-vm-modules yarn run jest --config jest.config.cjs --testTimeout 100000", | ||
"test:int": "NODE_OPTIONS=--experimental-vm-modules jest --testPathPattern=\\.int\\.test.ts --testTimeout 100000 --maxWorkers=50%", | ||
"test:standard:unit": "NODE_OPTIONS=--experimental-vm-modules jest --testPathPattern=\\.standard\\.test.ts --testTimeout 100000 --maxWorkers=50%", | ||
"test:standard:int": "NODE_OPTIONS=--experimental-vm-modules jest --testPathPattern=\\.standard\\.int\\.test.ts --testTimeout 100000 --maxWorkers=50%", | ||
"test:standard": "yarn test:standard:unit && yarn test:standard:int", | ||
"format": "prettier --config .prettierrc --write \"src\"", | ||
"format:check": "prettier --config .prettierrc --check \"src\"", | ||
"move-cjs-to-dist": "yarn lc-build --config ./langchain.config.js --move-cjs-dist", | ||
"create-entrypoints": "yarn lc-build --config ./langchain.config.js --create-entrypoints", | ||
"check-tree-shaking": "yarn lc-build --config ./langchain.config.js --tree-shaking" | ||
"format:check": "prettier --config .prettierrc --check \"src\"" | ||
}, | ||
"author": "LangChain", | ||
"license": "MIT", | ||
|
@@ -47,6 +43,7 @@ | |
"@cloudflare/workers-types": "^4.20231218.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey there! 👋 Just wanted to flag that the addition of "@langchain/standard-tests" as a dependency in the package.json file might impact the dev dependencies of the project. This is for the maintainers to review and ensure it aligns with the project's requirements. Thank you! |
||
"@jest/globals": "^29.5.0", | ||
"@langchain/scripts": "~0.0.14", | ||
"@langchain/standard-tests": "workspace:*", | ||
"@swc/core": "^1.3.90", | ||
"@swc/jest": "^0.2.29", | ||
"@tsconfig/recommended": "^1.0.3", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
/* eslint-disable no-process-env */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey team, just a heads up that I've flagged the latest change in the PR for review. It includes explicit checks for environment variables using |
||
import { test, expect } from "@jest/globals"; | ||
import { ChatModelIntegrationTests } from "@langchain/standard-tests"; | ||
import { AIMessageChunk } from "@langchain/core/messages"; | ||
import { | ||
ChatCloudflareWorkersAI, | ||
ChatCloudflareWorkersAICallOptions, | ||
} from "../chat_models.js"; | ||
|
||
class ChatCloudflareWorkersAIStandardIntegrationTests extends ChatModelIntegrationTests< | ||
ChatCloudflareWorkersAICallOptions, | ||
AIMessageChunk | ||
> { | ||
constructor() { | ||
if ( | ||
!process.env.CLOUDFLARE_ACCOUNT_ID || | ||
!process.env.CLOUDFLARE_API_TOKEN | ||
) { | ||
throw new Error( | ||
"Skipping Cloudflare Workers AI integration tests because CLOUDFLARE_ACCOUNT_ID or CLOUDFLARE_API_TOKEN is not set" | ||
); | ||
} | ||
super({ | ||
Cls: ChatCloudflareWorkersAI, | ||
chatModelHasToolCalling: false, | ||
chatModelHasStructuredOutput: false, | ||
constructorArgs: {}, | ||
}); | ||
} | ||
|
||
async testUsageMetadataStreaming() { | ||
this.skipTestMessage( | ||
"testUsageMetadataStreaming", | ||
"ChatCloudflareWorkersAI", | ||
"Streaming tokens is not currently supported." | ||
); | ||
} | ||
|
||
async testUsageMetadata() { | ||
this.skipTestMessage( | ||
"testUsageMetadata", | ||
"ChatCloudflareWorkersAI", | ||
"Usage metadata tokens is not currently supported." | ||
); | ||
} | ||
} | ||
|
||
const testClass = new ChatCloudflareWorkersAIStandardIntegrationTests(); | ||
|
||
test("ChatCloudflareWorkersAIStandardIntegrationTests", async () => { | ||
const testResults = await testClass.runTests(); | ||
expect(testResults).toBe(true); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/* eslint-disable no-process-env */ | ||
import { test, expect } from "@jest/globals"; | ||
import { ChatModelUnitTests } from "@langchain/standard-tests"; | ||
import { AIMessageChunk } from "@langchain/core/messages"; | ||
import { LangSmithParams } from "@langchain/core/language_models/chat_models"; | ||
import { | ||
ChatCloudflareWorkersAI, | ||
ChatCloudflareWorkersAICallOptions, | ||
} from "../chat_models.js"; | ||
|
||
class ChatCloudflareWorkersAIStandardUnitTests extends ChatModelUnitTests< | ||
ChatCloudflareWorkersAICallOptions, | ||
AIMessageChunk | ||
> { | ||
constructor() { | ||
super({ | ||
Cls: ChatCloudflareWorkersAI, | ||
chatModelHasToolCalling: false, | ||
chatModelHasStructuredOutput: false, | ||
constructorArgs: {}, | ||
}); | ||
} | ||
|
||
testChatModelInitApiKey() { | ||
this.skipTestMessage( | ||
"testChatModelInitApiKey", | ||
"ChatCloudflareWorkersAI", | ||
this.multipleApiKeysRequiredMessage | ||
); | ||
} | ||
|
||
expectedLsParams(): Partial<LangSmithParams> { | ||
console.warn( | ||
"Overriding testStandardParams. ChatCloudflareWorkersAI does not support temperature or max tokens." | ||
); | ||
return { | ||
ls_provider: "string", | ||
ls_model_name: "string", | ||
ls_model_type: "chat", | ||
ls_stop: ["Array<string>"], | ||
}; | ||
} | ||
} | ||
|
||
const testClass = new ChatCloudflareWorkersAIStandardUnitTests(); | ||
|
||
test("ChatCloudflareWorkersAIStandardUnitTests", () => { | ||
const testResults = testClass.runTests(); | ||
expect(testResults).toBe(true); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,10 +16,6 @@ | |
"scripts": { | ||
"build": "yarn turbo:command build:internal --filter=@langchain/cohere", | ||
"build:internal": "yarn lc-build:v2 --create-entrypoints --pre --tree-shaking", | ||
"build:esm": "NODE_OPTIONS=--max-old-space-size=4096 tsc --outDir dist/ && rm -rf dist/tests dist/**/tests", | ||
"build:cjs": "NODE_OPTIONS=--max-old-space-size=4096 tsc --outDir dist-cjs/ -p tsconfig.cjs.json && yarn move-cjs-to-dist && rm -rf dist-cjs", | ||
"build:watch": "yarn create-entrypoints && tsc --outDir dist/ --watch", | ||
"build:scripts": "yarn create-entrypoints && yarn check-tree-shaking", | ||
"lint:eslint": "NODE_OPTIONS=--max-old-space-size=4096 eslint --cache --ext .ts,.js src/", | ||
"lint:dpdm": "dpdm --exit-code circular:1 --no-warning --no-tree src/*.ts src/**/*.ts", | ||
"lint": "yarn lint:eslint && yarn lint:dpdm", | ||
|
@@ -30,11 +26,11 @@ | |
"test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch --testPathIgnorePatterns=\\.int\\.test.ts", | ||
"test:single": "NODE_OPTIONS=--experimental-vm-modules yarn run jest --config jest.config.cjs --testTimeout 100000", | ||
"test:int": "NODE_OPTIONS=--experimental-vm-modules jest --testPathPattern=\\.int\\.test.ts --testTimeout 100000 --maxWorkers=50%", | ||
"test:standard:unit": "NODE_OPTIONS=--experimental-vm-modules jest --testPathPattern=\\.standard\\.test.ts --testTimeout 100000 --maxWorkers=50%", | ||
"test:standard:int": "NODE_OPTIONS=--experimental-vm-modules jest --testPathPattern=\\.standard\\.int\\.test.ts --testTimeout 100000 --maxWorkers=50%", | ||
"test:standard": "yarn test:standard:unit && yarn test:standard:int", | ||
"format": "prettier --config .prettierrc --write \"src\"", | ||
"format:check": "prettier --config .prettierrc --check \"src\"", | ||
"move-cjs-to-dist": "yarn lc-build --config ./langchain.config.js --move-cjs-dist", | ||
"create-entrypoints": "yarn lc-build --config ./langchain.config.js --create-entrypoints", | ||
"check-tree-shaking": "yarn lc-build --config ./langchain.config.js --tree-shaking" | ||
"format:check": "prettier --config .prettierrc --check \"src\"" | ||
}, | ||
"author": "LangChain", | ||
"license": "MIT", | ||
|
@@ -45,6 +41,7 @@ | |
"devDependencies": { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey there! I noticed that a new devDependency "@langchain/standard-tests" has been added to the package.json file. This change is being flagged for your review to ensure it aligns with the project's dependency management strategy. Keep up the great work! |
||
"@jest/globals": "^29.5.0", | ||
"@langchain/scripts": "~0.0.14", | ||
"@langchain/standard-tests": "workspace:*", | ||
"@swc/core": "^1.3.90", | ||
"@swc/jest": "^0.2.29", | ||
"@tsconfig/recommended": "^1.0.3", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
/* eslint-disable no-process-env */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey team, just a heads up that I've flagged the latest change in the PR for your review. It looks like the code is explicitly accessing the |
||
import { test, expect } from "@jest/globals"; | ||
import { ChatModelIntegrationTests } from "@langchain/standard-tests"; | ||
import { AIMessageChunk } from "@langchain/core/messages"; | ||
import { ChatCohere, CohereChatCallOptions } from "../chat_models.js"; | ||
|
||
class ChatCohereStandardIntegrationTests extends ChatModelIntegrationTests< | ||
CohereChatCallOptions, | ||
AIMessageChunk | ||
> { | ||
constructor() { | ||
if (!process.env.COHERE_API_KEY) { | ||
throw new Error( | ||
"Can not run Cohere integration tests because COHERE_API_KEY is not set" | ||
); | ||
} | ||
super({ | ||
Cls: ChatCohere, | ||
chatModelHasToolCalling: false, | ||
chatModelHasStructuredOutput: false, | ||
constructorArgs: {}, | ||
}); | ||
} | ||
|
||
async testUsageMetadataStreaming() { | ||
this.skipTestMessage( | ||
"testUsageMetadataStreaming", | ||
"ChatCohere", | ||
"Streaming tokens is not currently supported." | ||
); | ||
} | ||
|
||
async testUsageMetadata() { | ||
this.skipTestMessage( | ||
"testUsageMetadata", | ||
"ChatCohere", | ||
"Usage metadata tokens is not currently supported." | ||
); | ||
} | ||
} | ||
|
||
const testClass = new ChatCohereStandardIntegrationTests(); | ||
|
||
test("ChatCohereStandardIntegrationTests", async () => { | ||
const testResults = await testClass.runTests(); | ||
expect(testResults).toBe(true); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there! 👋 I noticed that this PR adds a new dependency "@langchain/standard-tests". This comment is just to flag this change for the maintainers to review, in case it affects any peer/dev/hard dependencies. Keep up the great work! 🚀