Skip to content
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

fix(coverage): ignore urls from doc testing #25736

Merged
merged 6 commits into from
Sep 20, 2024

Conversation

kt3k
Copy link
Member

@kt3k kt3k commented Sep 19, 2024

We started executing doc examples in #25220. Because of that change, now the coverage data for generated urls like file:///path/to/module.ts$20-30.ts are generated, and deno coverage fails by trying to fetch that file from the cache. This behavior causes #25732

This PR fixes the above by ignoring urls of the pattern \$\d+-\d+\.(ts|js)$ when collecting the coverage data.

closes #25732

@kt3k kt3k added the ci-draft Run the CI on draft PRs. label Sep 19, 2024
@@ -452,6 +452,10 @@ fn filter_coverages(
let exclude: Vec<Regex> =
exclude.iter().map(|e| Regex::new(e).unwrap()).collect();

// Matches virtual file paths for doc testing
// e.g. file:///path/to/mod.ts$23-29.ts
let doc_test_re = Regex::new(r"\$\d+-\d+\.(ts|js)$").unwrap();
Copy link
Member Author

Choose a reason for hiding this comment

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

@magurotuna Do you know what are the possible file extensions from doc example extraction?

Copy link
Member

Choose a reason for hiding this comment

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

The extension is taken from the code fence, like:

```ts
// becomes .ts
```

```tsx
// becomes .tsx
```

Possible extensions are listed here
https://deno-docs--magurotuna-improved-doc-te.deno.dev/runtime/fundamentals/testing/#documentation-tests

  • js
  • javascript
  • mjs
  • cjs
  • jsx
  • ts
  • typescript
  • mts
  • cts
  • tsx

Copy link
Member Author

@kt3k kt3k Sep 20, 2024

Choose a reason for hiding this comment

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

Thanks! Updated

@kt3k kt3k marked this pull request as ready for review September 19, 2024 16:33
Copy link
Member

@magurotuna magurotuna left a comment

Choose a reason for hiding this comment

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

LGTM

@kt3k kt3k merged commit 66fb81e into denoland:main Sep 20, 2024
17 checks passed
@kt3k kt3k deleted the ignore-doc-testing-urls-in-coverage branch September 20, 2024 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-draft Run the CI on draft PRs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: deno coverage unexpectedly failing on simple snippet
2 participants