Skip to content

Commit b4a723f

Browse files
authoredSep 11, 2024
test: replace node-native APIs with ava for testing (copilot-extensions#78)
## Summary Towards copilot-extensions#73 This PR replaces node-native APIs with `ava` for testing. It does not tackle the domain of coverage (replacing to `c8`), as I think that would be better handled in its own PR. Here's a summary of changes: - Updates all files in `test/` to use `ava` instead of `node:test`, using `ava`'s built-in assertion methods - Revises the snapshot structure to use [ava's snapshot testing](https://github.com/avajs/ava/blob/main/docs/04-snapshot-testing.md) rather than the previously generated ones by Node's experimental feature - Updates `package.json`'s testing scripts to reflect the new `ava` reality - Commits the changes from `npm run lint:fix` for style consistency ### Questions for reviewers 👓 - Snapshots - Is the new snapshot structure clear and easy to follow? - Are we okay with the changes in `package.json` for the `test:code:update-snapshots` script? (More on [`--update-snapshots` here](https://github.com/avajs/ava/blob/main/docs/04-snapshot-testing.md).) - Testing - Metadata scope: In copilot-extensions@3871f77, I removed the `request` and `response` properties from the expectation to "fit" what [`ava`'s `.throwsAsync()`](https://github.com/avajs/ava/blob/main/docs/03-assertions.md#throwsasyncthrower-expectation-message) expects. Do folks prefer that we continue to support the metadata in the `request-response` pair as a part of the test, or is checking the `name` and `message` "enough" for what we're looking to check? - Deep Equality Checks: `ava` has [its version of `deepEqual()`](https://github.com/avajs/ava/blob/main/docs/03-assertions.md#deepequalactual-expected-message) which is powered by [Concordance](https://github.com/concordancejs/concordance). I used it as a replacement for the previous calls to [Node's `.assertStrictDeepEqual`](https://nodejs.org/api/assert.html#assertdeepstrictequalactual-expected-message). Reviewing [Concordance's Compare Implementation](https://github.com/concordancejs/concordance/blob/main/lib/compare.js), it would seem as though strict equality is enforced throughout and may be logically equivalent to what Node had already provided. Despite the tests passing, I wanted to flag that in the case we wanted to continue using Node's version still? 💭
1 parent 9ba5e44 commit b4a723f

11 files changed

+2649
-798
lines changed
 

‎index.d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ export interface CopilotMessage {
116116
[key: string]: unknown;
117117
}
118118

119-
export type MessageRole = 'system' | 'user' | 'assistant';
119+
export type MessageRole = "system" | "user" | "assistant";
120120
export interface InteropMessage<TRole extends MessageRole = MessageRole> {
121121
role: TRole;
122122
content: string;

‎index.test-d.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ export function transformPayloadForOpenAICompatibilityTest(
168168
expectType<{
169169
messages: {
170170
content: string;
171-
role: 'system' | 'user' | 'assistant';
171+
role: "system" | "user" | "assistant";
172172
name?: string;
173173
[key: string]: unknown;
174174
}[];

0 commit comments

Comments
 (0)
Failed to load comments.