-
Notifications
You must be signed in to change notification settings - Fork 634
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
docs(testing): improve the docs of @std/testing
#5033
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5033 +/- ##
==========================================
+ Coverage 92.10% 92.12% +0.01%
==========================================
Files 487 487
Lines 38798 38865 +67
Branches 5432 5435 +3
==========================================
+ Hits 35736 35805 +69
+ Misses 3006 3005 -1
+ Partials 56 55 -1 ☔ View full report in Codecov by Sentry. |
@@ -67,7 +72,8 @@ const ENTRY_POINTS = [ | |||
] as const; | |||
|
|||
const TS_SNIPPET = /```ts[\s\S]*?```/g; | |||
const ASSERTION_IMPORT = /import \{.*\} from "@std\/assert(?:\/.*)?";/gm; | |||
const ASSERTION_IMPORT = | |||
/from "@std\/(assert(\/[a-z-]+)?|testing\/(mock|snapshot|types))"/g; |
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.
Removed import \{.*\}
part as this doesn't work if there are line breaks between import
and from
.
Also added testing\/(mock|snapshot|types)
part as these files also include assertion functions.
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.
Having assertions spread across multiple packages is confusing. What if we moved assertions (only) to @std/assert
? WDYT?
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.
That makes @std/assert
a huge package. In the current layout, assertions are organized by the category (mock / snapshot / types). This makes more sense to me.
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.
First pass review
testing/_test_suite.ts
Outdated
@@ -62,6 +64,7 @@ const optionalTestStepDefinitionKeys: (keyof Deno.TestStepDefinition)[] = [ | |||
* A group of tests. | |||
*/ | |||
export interface TestSuite<T> { | |||
/** The symbol */ |
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 doesn't seem informative.
testing/types.ts
Outdated
* @typeParam T The expected type (`true` or `false`) | ||
* @param expectTrue - True if the passed in type argument resolved to true. | ||
*/ | ||
export function assertType<T extends true | false>(_expectTrue: T) { | ||
} | ||
export function assertType<T extends true | false>( | ||
// deno-lint-ignore no-unused-vars |
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.
* @typeParam T The expected type (`true` or `false`) | |
* @param expectTrue - True if the passed in type argument resolved to true. | |
*/ | |
export function assertType<T extends true | false>(_expectTrue: T) { | |
} | |
export function assertType<T extends true | false>( | |
// deno-lint-ignore no-unused-vars | |
* @typeParam T The expected type | |
* @param expectTrue - True if the passed in type argument resolved to true. | |
*/ | |
export function assertType<T extends boolean>( | |
// deno-lint-ignore no-unused-vars |
testing/bdd.ts
Outdated
@@ -816,6 +1023,7 @@ describe.only = function describeOnly<T>( | |||
}); | |||
}; | |||
|
|||
/** Ignore the test suite */ |
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 we include targeted examples for these methods?
@@ -67,7 +72,8 @@ const ENTRY_POINTS = [ | |||
] as const; | |||
|
|||
const TS_SNIPPET = /```ts[\s\S]*?```/g; | |||
const ASSERTION_IMPORT = /import \{.*\} from "@std\/assert(?:\/.*)?";/gm; | |||
const ASSERTION_IMPORT = | |||
/from "@std\/(assert(\/[a-z-]+)?|testing\/(mock|snapshot|types))"/g; |
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.
Having assertions spread across multiple packages is confusing. What if we moved assertions (only) to @std/assert
? WDYT?
@std/testing
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.
Great work! Good to see this done.
* }, Error) | ||
* ``` | ||
* @param value The value to set | ||
*/ | ||
set start(value: number) { |
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 remove this setter in a follow-up PR.
part of #3764