-
Notifications
You must be signed in to change notification settings - Fork 218
ci: run tests with hono resolution matrix #1086
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
ci: run tests with hono resolution matrix #1086
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1086 +/- ##
==========================================
- Coverage 79.48% 79.28% -0.21%
==========================================
Files 77 77
Lines 2276 2278 +2
Branches 574 574
==========================================
- Hits 1809 1806 -3
- Misses 391 396 +5
Partials 76 76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e0339bc
to
a16eb15
Compare
|
102a3ab
to
57075a8
Compare
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.
Sorry for the all of the CI failures earlier @yusukebe , I had meant to create this PR in my own fork first 😅
If you think this PR will be useful, and a good direction to take, I can update the remaining workflows.
Due to existing issue with different workspaces using different versions of hono, I'm happy to focus on the workspaces that work, and fix the failing types or tests in future PRs
strategy: | ||
matrix: | ||
hono-version: [hono@^3.0.0] | ||
# hono-version: ['hono@^3.0.0', 'hono@^4.0.0'] |
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.
Unsure how useful this might be.. but I noticed different workspaces reference different versions of hono, so I thought it might be helpful to run CI against them
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: actions/setup-node@v4 | ||
with: | ||
node-version: 20.x | ||
- run: yarn add ${{ matrix.hono-version }} --mode=update-lockfile |
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.
Start by adding the current hono version, but do not install, just update the lock file
steps: | ||
- uses: actions/checkout@v4 | ||
- uses: actions/setup-node@v4 | ||
with: | ||
node-version: 20.x | ||
- run: yarn add ${{ matrix.hono-version }} --mode=update-lockfile | ||
- run: yarn workspaces focus hono-middleware @hono/arktype-validator |
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.
yarn workspaces focus
will install the dependencies of the focused workspaces, including the selected version of hono
- run: yarn workspaces focus hono-middleware @hono/zod-openapi | ||
- run: yarn workspace @hono/zod-openapi build | ||
- run: yarn workspaces foreach --topological --recursive --from @hono/zod-openapi run build |
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.
Recessively build the dependencies of the current workspace
This is specifically for @hono/zod-openapi
as it has a dependency on @hono/zod-validator
which needs building first
@@ -68,6 +68,7 @@ | |||
"@typescript-eslint/parser": "^8.7.0", | |||
"@vitest/coverage-istanbul": "^3.0.8", | |||
"eslint": "^9.17.0", | |||
"hono": "*", |
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.
Setting a single version in the root is enough to satisfy the peer dependencies of each workspace. It also ensures that there's only a single version installed at any one time.
@@ -44,7 +44,6 @@ | |||
"devDependencies": { | |||
"@arethetypeswrong/cli": "^0.17.4", | |||
"ajv": ">=8.12.0", | |||
"hono": "^4.4.12", |
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.
Can remove any reference to hono in dev dependencies as the single reference in the root package.json is enough to satisfy the peer dependency
@@ -52,7 +51,7 @@ | |||
}, | |||
"dependencies": { | |||
"@asteasolutions/zod-to-openapi": "^7.1.0", | |||
"@hono/zod-validator": "npm:0.4.2" | |||
"@hono/zod-validator": "workspace:^" |
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.
Now that CI builds dependencies of a workspace, the workspace:
protocol can be used to reference @hono/zod-validator
directly instead of from npm
5e4ea45
to
e2b4f73
Compare
e2b4f73
to
83bec6d
Compare
In my opinion, this PR is unfortunately unnecessary because middleware basically only needs to support |
Ah.. you're right. We even discussed it in the original issue #990 (comment) 😅 I'll focus on |
Thank you so much for understanding! |
Run CI against multiple versions of hono using matrix strategy