Skip to content

fix(test): handle snippet containing Deno.test in doc test #29631

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

Merged
merged 1 commit into from
Jun 10, 2025

Conversation

scarf005
Copy link
Contributor

@scarf005 scarf005 commented Jun 6, 2025

fixes #29629

skips wrapping code snippet in Deno.test if it already has one.

# Title

```ts
import { assertEquals } from "@std/assert/equals";

Deno.test("add", () => {
  assertEquals(1 + 2, 3);
});
```

this feature is useful when doctest itself uses Deno.test inside the snippet, such as https://docs.deno.com/examples/mocking_tutorial/.

known issues: will false-positive on Deno.test inside multiline comments but such cases should be rare

@scarf005 scarf005 force-pushed the feat/doctest-extract-as-file branch from 2dc9feb to d86fd22 Compare June 6, 2025 02:40
@kt3k
Copy link
Member

kt3k commented Jun 6, 2025

This option sounds confusing to me. How about detecting the usage of Deno.test automatically and avoid wrapping when Deno.test is used (I think it's ok for the detection to have false positives and false negatives if it supports main use cases)

@scarf005
Copy link
Contributor Author

scarf005 commented Jun 6, 2025

@kt3k could i get pointers on how to detect Deno.test?

@kt3k
Copy link
Member

kt3k commented Jun 6, 2025

Deno.test should be used at top-level or at least top-level of blocks in most practical cases. I think it's practically fine to detect it by the regexp of /^\s*Deno.\.test\(/ for each line.

@scarf005 scarf005 force-pushed the feat/doctest-extract-as-file branch from d86fd22 to 9c90317 Compare June 6, 2025 09:02
@scarf005
Copy link
Contributor Author

scarf005 commented Jun 6, 2025

@kt3k applied and force-pushed in be81dba (#29631)

@scarf005 scarf005 changed the title feat(cli): option to not wrap snippets in Deno.test in doc test fix(cli): run snippet containing Deno.test in doc test Jun 6, 2025
@scarf005 scarf005 changed the title fix(cli): run snippet containing Deno.test in doc test fix(cli): handle snippet containing Deno.test in doc test Jun 6, 2025
@scarf005 scarf005 force-pushed the feat/doctest-extract-as-file branch from 9c90317 to 67b7a32 Compare June 6, 2025 09:06
@scarf005 scarf005 force-pushed the feat/doctest-extract-as-file branch from 67b7a32 to be81dba Compare June 6, 2025 09:13
Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

I also like to hear opinions from @bartlomieju @dsherret @nathanwhit

@kt3k kt3k changed the title fix(cli): handle snippet containing Deno.test in doc test fix(test): handle snippet containing Deno.test in doc test Jun 9, 2025
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems quite niche, but I'm fine with this solution, +1 from me

Copy link
Member

@dsherret dsherret left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM too

@kt3k kt3k merged commit baa52bf into denoland:main Jun 10, 2025
18 checks passed
@scarf005 scarf005 deleted the feat/doctest-extract-as-file branch June 10, 2025 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deno test --doc does not test Deno.test in code block
4 participants