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

move testing to vitest #83

Merged
merged 1 commit into from
Nov 4, 2024
Merged

move testing to vitest #83

merged 1 commit into from
Nov 4, 2024

Conversation

bmschmidt
Copy link
Collaborator

@bmschmidt bmschmidt commented Nov 1, 2024

Still failing a test on wait_for_project_lock.


Important

Migrate testing framework from uvu to vitest, updating test scripts, dependencies, and test files accordingly.

  • Testing Framework Migration:
    • Replace uvu with vitest in package.json test scripts.
    • Remove uvu and related dependencies; add vitest.
    • Add vitest.config.ts for configuration.
  • Test Files:
    • Convert test files from .js to .ts (e.g., arrow.test.js to arrow.ts).
    • Update test syntax from uvu to vitest in embedding.test.ts, neighbors.test.ts, project.test.ts, and user.test.ts.
    • Replace assert with expect in test assertions.
  • Miscellaneous:
    • Remove project.test.js and replace with project.test.ts with updated test cases.

This description was created by Ellipsis for ecd2422. It will automatically update as commits are pushed.

Copy link
Collaborator Author

bmschmidt commented Nov 1, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @bmschmidt and the rest of your teammates on Graphite Graphite

@bmschmidt bmschmidt marked this pull request as ready for review November 1, 2024 17:35
'Index actually builds and is returned from server',
async () => {
await project.wait_for_lock();
index = (await project.indices())[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Add error handling for the API call to project.indices() to ensure robustness in case of failures.

@@ -1,7 +1,6 @@
import { test } from 'uvu';
import { test } from 'vitest';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import { test } from 'vitest';
import { test, expect } from 'vitest';

@@ -16,7 +15,7 @@ test('Embed a point from an env token', async () => {
// Pulling the API key from env variable.
const values = await embed(strings);
Copy link
Contributor

Choose a reason for hiding this comment

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

API calls should include error handling to manage potential failures.

Copy link
Collaborator

Choose a reason for hiding this comment

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

no new import?

Copy link
Collaborator

@RLesser RLesser left a comment

Choose a reason for hiding this comment

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

LGTM, address human and bot comments

should we add a CI check for test passing? and isn't there a tsc check as well?

Copy link
Collaborator Author

bmschmidt commented Nov 4, 2024

Merge activity

  • Nov 4, 10:02 AM EST: A user started a stack merge that includes this pull request via Graphite.
  • Nov 4, 10:02 AM EST: A user merged this pull request with Graphite.

@bmschmidt bmschmidt merged commit 528867c into main Nov 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants