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

test(node/fs): add fs.Dir tests #18463

Merged
merged 8 commits into from
Mar 28, 2023

Conversation

kamalsingh200238
Copy link
Contributor

Part of #17840

I haven't made any changes or additions to the tests themselves, just moved the tests over and updated to match.

_fs tests which have been ported: _fs_dir_test.ts

@CLAassistant
Copy link

CLAassistant commented Mar 27, 2023

CLA assistant check
All committers have signed the CLA.

@bartlomieju bartlomieju requested a review from kt3k March 27, 2023 16:26
@aapoalas aapoalas changed the title Kamalsingh200238 fs tests test(node/FS): fs tests Mar 27, 2023
@aapoalas aapoalas changed the title test(node/FS): fs tests test(node/fs): fs tests Mar 27, 2023
Comment on lines 8 to 9
import Dir from "ext:deno_node/_fs/_fs_dir.ts";
import type Dirent from "ext:deno_node/_fs/_fs_dirent.ts";
Copy link
Member

Choose a reason for hiding this comment

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

ext: only works for the internal Deno code. I think these should be imported like the below:

Suggested change
import Dir from "ext:deno_node/_fs/_fs_dir.ts";
import type Dirent from "ext:deno_node/_fs/_fs_dirent.ts";
import { Dir, type Dirent } from "node:fs";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for helping out, I have made the changes according to your suggestion.
Please have a look.

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.

Left one comment. Other part looks good to me

@kt3k kt3k changed the title test(node/fs): fs tests test(node/fs): add fs.Dir tests Mar 28, 2023
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

@kt3k kt3k merged commit e789f81 into denoland:main Mar 28, 2023
@kt3k kt3k mentioned this pull request Mar 28, 2023
55 tasks
mmastrac pushed a commit that referenced this pull request Mar 31, 2023
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.

3 participants