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

feat: [Issue-185] Token Provider Tests #365

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

normand1
Copy link

@normand1 normand1 commented Nov 16, 2024

Relates to:

#185

Risks

Low: Only modification to src code files was adding optional syntax an

Background

What does this PR do?

  • Removes jest in favor of vitest as discussed in this issue: Core Unit Tests #340
  • Fixes tests that were no longer running in packages/core/src/tests/token.test.ts

What kind of change is this?

Improvements - Tests

See Issue above

Documentation changes needed?

My changes do not require a change to the project documentation.

Testing

pnpm -w run test -- 'src/tests/token.test.ts'

Where should a reviewer start?

pnpm install
pnpm run build
pnpm -w run test -- 'src/tests/token.test.ts'

Detailed testing steps

@normand1 normand1 mentioned this pull request Nov 16, 2024
@normand1
Copy link
Author

@ponderingdemocritus This is based on your earlier instructions to include and use vitest:
#340 (comment)

@ponderingdemocritus ponderingdemocritus changed the title [Issue-185] Token Provider Tests feat: [Issue-185] Token Provider Tests Nov 17, 2024
},
"exports": {
".": "./dist/index.js",
"./sqlite_vec": "./dist/sqlite_vec.js"
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think this is needed - exported at index

import { Connection } from "@solana/web3.js";
import { PublicKey } from "@solana/web3.js";

export { TokenProvider, WalletProvider, Connection, PublicKey };
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the solana exports and import where needed from the packages

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, this was left over from another test I was working on, but ended up simplifying for this first PR

Copy link
Contributor

@ponderingdemocritus ponderingdemocritus left a comment

Choose a reason for hiding this comment

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

good start - see comments

@normand1
Copy link
Author

normand1 commented Nov 17, 2024

@ponderingdemocritus Thanks for the feedback! Just pushed an update based on comments

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