-
-
Notifications
You must be signed in to change notification settings - Fork 41
Feat: setup testing infrastructure #114
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: setup testing infrastructure #114
Conversation
175935c to
7bd9fda
Compare
7bd9fda to
d680ba1
Compare
| "dev": "pnpm run migrate && tsx watch --env-file-if-exists=.env --dns-result-order=ipv6first bin/server.ts", | ||
| "test": "tsx --env-file-if-exists=.env.test --test", | ||
| "check": "tsc && biome check .", | ||
| "migrate": "drizzle-kit migrate", |
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.
We probably need to use dotenv in drizzle config in order to support migrating the test database in development, in CI the need for dotenv is bypassed because we're setting the DATABASE_URL as an environment variable.
|
@dahlia as |
ae77cc9 to
4de5638
Compare
| @@ -0,0 +1,165 @@ | |||
| name: release | |||
|
|
|||
| on: | |||
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.
This workflow is identical to the build.yaml workflow that's been removed, the only change is that we're not running this on pull requests.
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.
We can probably delete the conditionals here on if it's a push.
dahlia
left a comment
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.
Thanks a lot! I left a few review comments.
| - "*.*.*" | ||
|
|
||
| jobs: | ||
| check: |
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.
@dahlia I'm not sure we even need check in this workflow, apart from the last step about something for tags and changes to be released?
dahlia
left a comment
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.
Everything looks good to me. Thank you!
This changes
src/index.tsxto not directly start the application, but instead to just create it, such that it can be required in different scenarios.The entrypoint
bin/server.tsis now used instead, and I've also added support for specifying theLISTEN_HOST(this may currently be backwards incompatible, as Hollo may have been listening on all interfaces originally).There is an issue here in that we can't currently migrate the "test" database because
drizzle-kitdoesn't seem to accept any way to pass the correct env-file to use, the documentation seems to suggest usingdotenvinstead: https://orm.drizzle.team/docs/get-started/postgresql-newWe also currently use the Flydrive-js
Diskinterface, but for testing we'd want to switch toDriveManagerwhich supports fakes: https://flydrive.dev/docs/drive_manager (this means we don't actually need to test against s3 or fs disks), for now, I'm using thefsdriver and creating a directory before running the tests. Issue #115 addresses this.