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

chore: stop cli tests from reading server source #6711

Closed
wants to merge 8 commits into from

Conversation

benmccann
Copy link
Contributor

as suggested in #6678 (comment). this should unblock the vitest migration for the CLI project

@etnoy
Copy link
Contributor

etnoy commented Jan 29, 2024

Looks good for the most part, but do we really want to move the api e2e to the tests folder?

@etnoy
Copy link
Contributor

etnoy commented Jan 29, 2024

Node versions below 20.8 usually core dump when running tests, if you want a hint for debugging the failing test

@jrasm91
Copy link
Contributor

jrasm91 commented Jan 29, 2024

#6719 should fix the core dump issue btw. No need to specify the node version, etc.

@benmccann
Copy link
Contributor Author

Looks good for the most part, but do we really want to move the api e2e to the tests folder?

We need the e2e api to be compiled because it's shared by the server and CLI projects. Currently the CLI project depends on the e2e api source code, which means the CLI project depends on the source code of the whole server project rather than the compiled output. We can put it in a different directory like src/e2e instead of src/test if you prefer, but it does have to move to the src folder to get compiled or else we can't have the CLI project use it.

Node versions below 20.8 usually core dump when running tests, if you want a hint for debugging the failing test

Thanks! The tests are passing now by switching to Node 20

#6719 should fix the core dump issue btw. No need to specify the node version, etc.

Sure, that works too. Though I was hoping to replace Jest with Vitest (#6678) after this PR is merged, which would let us run on Node 18 again without issue or needing to mock anything.

@benmccann
Copy link
Contributor Author

closing in favor of #6747

@benmccann benmccann closed this Jan 30, 2024
@benmccann benmccann deleted the restructure branch January 30, 2024 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants