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(cli): handle edge cases around exports in doc tests and default export #25720

Merged
merged 3 commits into from
Sep 19, 2024

Conversation

magurotuna
Copy link
Member

@magurotuna magurotuna commented Sep 19, 2024

This commit fixes issues with the pseudo test file generation logic, namely:

  • exports declared in snippets
  • auto import insertion for default export

Case 1: exports declared in snippets

In the previous implementation, exports declared in snippets were moved to the top level of the module in the generated pseudo test file. This is required because export must be at the top level.

This becomes a problem if such a export has a body, containing a reference to a local variable. Suppose we extract this snippet from JSDoc:

const logger = createLogger("my-awesome-module");

export function sum(a: number, b: number): number {
  logger.debug("sum called");
  return a + b;
}

This gets converted into the following invalid code (note that export function sum is moved to the top level, but its body references logger variable which can't be referenced from here):

export function sum(a: number, b: number): number {
  logger.debug("sum called");
  return a + b;
}

Deno.test("./base.ts$1-7.ts", async () => {
  const logger = createLogger("my-awesome-module");
});

To resolve this issue, this commit adds a logic to remove the export keyword, allowing the exported items to stay in the Deno.test block scope, like so:

Deno.test("./base.ts$1-7.ts", async () => {
  const logger = createLogger("my-awesome-module");

  function sum(a: number, b: number): number {
    logger.debug("sum called");
    return a + b;
  }
});

Case 2: default export

Previously default export foo was not captured by the export collector, so auto import insertion didn't work for this case. To put it concretely, the following code snippet didn't work when run with deno test --doc because import foo from "file:///path/to/mod.ts" didn't get inserted automatically:

/**
 * ```ts
 * console.log(foo);
 * ```
 * 
 * @module
 */

const foo = 42;
export default foo;

This commit fixes this issue and the above example works fine.


Fixes #25718

@magurotuna
Copy link
Member Author

@iuioiua FYI, with this patch the result of running deno test --doc on std/log/mod.ts is like this:

❯ ~/Repo/github.com/magurotuna/deno/target/debug/deno test --doc --no-lock log/mod.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$30-42.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$47-56.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$78-95.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$121-131.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$133-189.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$192-241.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$245-279.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$282-311.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$314-327.ts
Check file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$330-354.ts
running 0 tests from ./log/mod.ts
running 1 test from ./log/mod.ts$30-42.ts
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$30-42.ts ... ok (0ms)
running 1 test from ./log/mod.ts$47-56.ts
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$47-56.ts ...
------- post-test output -------
INFO This is the message
INFO {"thisWontBe":"JSON.stringify'd"}
----- post-test output end -----
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$47-56.ts ... ok (0ms)
running 1 test from ./log/mod.ts$78-95.ts
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$78-95.ts ... ok (0ms)
running 1 test from ./log/mod.ts$121-131.ts
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$121-131.ts ... ok (0ms)
running 1 test from ./log/mod.ts$133-189.ts
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$133-189.ts ...
------- post-test output -------
INFO 123456
WARN true
ERROR {"foo":"bar","fizz":"bazz"}
CRITICAL 500 Internal server error
----- post-test output end -----
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$133-189.ts ... FAILED (2ms)
running 1 test from ./log/mod.ts$192-241.ts
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$192-241.ts ...
------- post-test output -------
[DEBUG] Hello, world!
10 Hello, world!, arg0: 1, arg1: two, arg2: 3,4,5
[dataLogger] - ERROR oh no!
----- post-test output end -----
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$192-241.ts ... ok (0ms)
running 1 test from ./log/mod.ts$245-279.ts
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$245-279.ts ...
------- post-test output -------
{"level":"INFO","datetime":1726724031061,"message":"Hey"}
{"level":"INFO","datetime":1726724031061,"message":"Hey","args":{"product":"nail"}}
{"level":"INFO","datetime":1726724031061,"message":"Hey","args":[1,"two",[3,4,5]]}
----- post-test output end -----
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$245-279.ts ... ok (0ms)
running 1 test from ./log/mod.ts$282-311.ts
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$282-311.ts ...
------- post-test output -------
{"lvl":20,"msg":"complete","time":"2024-09-19T05:33:51.083Z","name":"default"}
----- post-test output end -----
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$282-311.ts ... ok (1ms)
running 1 test from ./log/mod.ts$314-327.ts
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$314-327.ts ...
------- post-test output -------
hello world
true
123
----- post-test output end -----
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$314-327.ts ... ok (0ms)
running 1 test from ./log/mod.ts$330-354.ts
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$330-354.ts ...
------- post-test output -------
undefined
----- post-test output end -----
file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$330-354.ts ... ok (0ms)

 ERRORS

file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$133-189.ts => ./log/mod.ts$133-189.ts:2:6
error: NotCapable: Requires write access to "./log.txt", run again with the --allow-write flag
    this[fileSymbol] = Deno.openSync(
                            ^
    at Object.openSync (ext:deno_fs/30_fs.js:525:15)
    at FileHandler.setup (file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/file_handler.ts:78:29)
    at Module.setup (file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/setup.ts:26:13)
    at file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$133-189.ts:11:9

 FAILURES

file:///Users/yusuke/Repo/github.com/magurotuna/deno_std/log/mod.ts$133-189.ts => ./log/mod.ts$133-189.ts:2:6

FAILED | 9 passed | 1 failed (224ms)

error: Test failed

This failure makes sense to me

@iuioiua
Copy link
Contributor

iuioiua commented Sep 19, 2024

Agreed. I ignore in my upcoming PR.

@magurotuna magurotuna merged commit 0ea71ab into denoland:main Sep 19, 2024
17 checks passed
@magurotuna magurotuna deleted the magurotuna/fix-doc-test branch September 19, 2024 07:19
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.

bug: deno test --doc incorrectly evaluates snippets
3 participants