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

Deno Doctest Support #6124

Closed
wants to merge 57 commits into from
Closed

Deno Doctest Support #6124

wants to merge 57 commits into from

Conversation

iykekings
Copy link

@iykekings iykekings commented Jun 5, 2020

This PR adds --docs to deno test sub-command for running tests on JSDoc @example blocks.

To use:

  • have code blocks in @example sections with three back-ticks as in markdown code blocks
  • deno test --docs runs all non-test files
  • Imports are done relative to root path, eg.

src/linkedlist.ts

export class Linkedlist<T>  {
// skipped lines
 /**
 * @param list - LinkedList<T>
 * @example <caption>Linkedlist.compareWith</caption>
 * import { LinkedList } from './src/linkedlist.ts';
 * const testArr = [1, 2, 3, 4, 5, 6, 78, 9, 0, 65];
 * const firstList = new LinkedList<number>();
 * const secondList = new LinkedList<number>();
 * for (let data of testArr) {
 *   firstList.insertNode(data);
 *   secondList.insertNode(data);
 * }
 * const result = firstList.compareWith(secondList);
 * assert.assertEquals(result, true);
 * @returns boolean
 */
  compareWith(list: LinkedList<T>): boolean {
    let current1 = this.head;
    let current2 = list.head;
    while (current1 && current2) {
      if (current1.data !== current2.data) return false;
      if (current1.next && !current2.next && !current1.next && current2.next) {
        return false;
      }
      current1 = current1.next;
      current2 = current2.next;
    }
    return true;
  }
}
  • All functions exported from https://deno.land/std/testing/asserts.ts can be used without importing them in an assert object. (assert.assert, assert.assertEquals, assert.assertNotEquals etc.)
  • await keyword can be used without wrapping in async function. NB: This is not due to TLA, but await will be detected in the example and the generated test is wrapped in an async function.
  • supports caption tag on @example block and used name of the test, by default test result is printed like this test src/linkedlist.ts - (line 160) ... ok (0ms), but with caption, like in the example above it will be test src/linkedlist.ts - Linkedlists.compareWith (line 160) ... ok (0ms)
  • Supports multiple @example blocks in one JSDoc comment

To ignore test:

  • wrap @example code block with back-ticks
  • Tag the three opening back-ticks with ignore, and it will be picked up but not tested, will be recorded by Deno test as ignored.
  • Tag the three opening back-ticks with text, and it will be seen as normal text

  • This is inspired by rustdoc --test

  • Example output

Screenshot 2020-10-09 at 14 05 41

Resolves #4716

@iykekings
Copy link
Author

@yaahc

cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/main.rs Outdated Show resolved Hide resolved
* fix clippy errors

* break extract_jsdoc_examples to smaller fns

* fix error when ran without arguments
@ry
Copy link
Member

ry commented Jun 6, 2020

This is a really cool feature. Thanks!

Rather than adding a new subcommand, can you make this "deno test --docs" instead?

We're releasing v1.1.0 on June 13 - hopefully we'll be able to get this in before then.

@ry ry added this to the 1.1.0 milestone Jun 6, 2020
@ry ry added cli related to cli/ dir feat new feature (which has been agreed to/accepted) labels Jun 6, 2020
@iykekings
Copy link
Author

Okay, l will do that.

let res = extract_jsdoc_examples(test.to_string(), PathBuf::from("user"));
assert!(res.is_none());
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice tests!

let mut import_set = HashSet::new();

let test_bodies = JS_DOC_PATTERN
.captures_iter(&input)
Copy link
Member

Choose a reason for hiding this comment

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

I'm worried about the use of regex here. I'm fine for using regex to extract example code from jsdoc comments, but we have a complete TypeScript parser (SWC) built-in. We should be using this to extract the JSDocs. There should be examples of this in cli/doc/

Copy link
Author

Choose a reason for hiding this comment

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

Currently there is no way to recursively loop through DocNodes returned by the parser to get all the JSDocs but @bartlomieju promises to include the feature by next week. Should I go on with this or hold off till that feature is ready?

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.

Impressive @iykekings! Here are initial thoughts on the patch. Please update manual entry for deno test describing this feature.

Lots of heavy lifting is done using regexes, I wonder if @dsherret's Markdown parser could be used in the future to make extracting the code more robust.

cli/js/testing.ts Outdated Show resolved Hide resolved
cli/flags.rs Outdated Show resolved Hide resolved
cli/flags.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
cli/doctest_runner.rs Outdated Show resolved Hide resolved
@dsherret
Copy link
Member

dsherret commented Jun 8, 2020

@bartlomieju I think we need a JSDoc parser in swc. I was thinking about it over the weekend. Opened swc-project/swc#827

@ry ry removed this from the 1.1.0 milestone Jun 10, 2020
@bartlomieju bartlomieju added this to the 1.2.0 milestone Jun 12, 2020
@iykekings
Copy link
Author

This PR is waiting for SWC to implement JSDoc parser(instead of using Regex). You can track the progress here swc-project/swc#829.

@bartlomieju bartlomieju removed this from the 1.2.0 milestone Jul 14, 2020
@iykekings
Copy link
Author

PR is ready, will try to fix the CI issues.
The line numbers printed in the tests are those of the jsdoc comments,
There is an issue open on swc to make it match the exact line of @example tags

@bartlomieju bartlomieju mentioned this pull request Oct 10, 2020
22 tasks
bartlomieju and others added 15 commits October 10, 2020 11:54
A recent change in rustc or cargo made it so that rusty_v8's `build.rs`,
which is responsible for downloading `librusty_v8.a`, does not get
rebuilt or re-run when its build output directory is restored from the
Github Actions cache.

However, rusty_v8's custom build script does not save the download to
its build output directory; it puts the file in
`target/debug|release/gn_out/obj` instead.

To get CI going again we opted to add `target/*/gn_out` to the Github
Actions cache.

A more robust fix would be make rusty_v8 save the download to the
cargo-designated output directory.
This commit rewrites deno::Worker to not implement Future
trait.

Instead there are two separate methods:
- Worker::poll_event_loop() - does single tick of event loop
- Worker::run_event_loop() - runs event loop to completion

Additionally some cleanup to Worker's field visibility was done.
Also re-export serde from deno_core, since its now a dependency.
* Revert "refactor: Worker is not a Future (denoland#7895)"

This reverts commit f4357f0.

* Revert "refactor(core): JsRuntime is not a Future (denoland#7855)"

This reverts commit d8879fe.

* Revert "fix(core): module execution with top level await (denoland#7672)"

This reverts commit c7c7677.
@iykekings
Copy link
Author

continued in #7916

@iykekings iykekings closed this Oct 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deno test should run jsdoc example code