-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Add meta
to TestOptions
#8405
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
base: main
Are you sure you want to change the base?
Conversation
TestOptions
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
TestOptions
meta
to TestOptions
|
Now cascades from grandparents |
packages/runner/src/types/tasks.ts
Outdated
/** | ||
* Custom metadata for the task. This will be merged with any meta property defined in the test. | ||
*/ | ||
meta?: Record<string, unknown> |
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 needs to be Partial<TaskMeta>
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.
Some places it is Record<string, unknown>
should these all be Partial<TaskMeta>
?
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.
e.g.
vitest/packages/vite-node/src/types.ts
Line 99 in c1ac15c
meta?: Record<string, any> | null |
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.
I don't know what to do about this. Setting Partial<TaskMeta>
cause a lot of errors now 😢 Does TaskMeta
want to be extends Record<string, any>
?
I don't understand complex typing much yet.
|
||
test('should allow adding meta at runtime', { meta: { testLevel: 'runtime-test' } }, ({ task }) => { | ||
// Add meta at runtime | ||
(task.meta as any).runtimeAdded = 'added-during-test' |
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.
Let's add a local type instead of assigning any
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.
With meta now being Partial<TaskMeta>
we can now assign the property directly without any types? is that okay?
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.
I was wrong 😿
@sheremet-va Thank you for your review! The comments on
So in this instance it can be left out? I shall fix this 😸 |
/** | ||
* Custom metadata for the task that will be assigned to `task.meta`. | ||
*/ | ||
meta?: Record<string, unknown> |
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.
@sheremet-va This is what I meant here
? 'todo' | ||
: 'run', | ||
meta: options.meta ?? Object.create(null), | ||
meta: { |
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.
Let's keep the meta with a null prototype. Just assign options.meta
to meta collected with collectAncestorMeta
:
const testMeta = collectAncestorMeta(collectorContext.currentSuite?.suite)
if(options.meta) {
Object.assign(testMeta, options.meta)
}
{
// ...
meta: testMeta,
}
shuffle: suiteOptions?.shuffle, | ||
tasks: [], | ||
meta: Object.create(null), | ||
meta: { |
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.
Same here
Description
Related issue: #8404
This PR allows assiging data to the
meta
context property viaTestOptions
. Properties assigned here will be available inbeforeEach
. Properties defined in the test body will override these, maintaining current behaviourPlease don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yaml
unless you introduce a new test example.Tests
pnpm test:ci
.Documentation
pnpm run docs
command.Changesets
feat:
,fix:
,perf:
,docs:
, orchore:
.