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

port remaining std/node unit test cases #17840

Closed
51 of 55 tasks
kt3k opened this issue Feb 21, 2023 · 21 comments
Closed
51 of 55 tasks

port remaining std/node unit test cases #17840

kt3k opened this issue Feb 21, 2023 · 21 comments
Labels
good first issue Good for newcomers help wanted community help requested

Comments

@kt3k
Copy link
Member

kt3k commented Feb 21, 2023

We need to port test from https://github.com/denoland/deno_std/tree/0.177.0/node

remaining:

@kt3k kt3k added good first issue Good for newcomers help wanted community help requested labels Feb 22, 2023
@maxedahlgren
Copy link
Contributor

Hi there, I'd like to contribute to this repository and feel like I may be able to help with this issue. It would be great if I could be pointed to where I could find the original test cases

@kt3k
Copy link
Member Author

kt3k commented Mar 16, 2023

Hi @maxedahlgren The original test cases are available node/ dir in v0.177.0 tag (the last tag we had std/node) https://github.com/denoland/deno_std/tree/0.177.0/node

@Farsen976
Copy link
Contributor

Farsen976 commented Mar 17, 2023

node/async_hooks_test.ts has been added by @bartlomieju #18004

andrewnester added a commit to andrewnester/deno that referenced this issue Mar 28, 2023
This PR ports timers_tests.ts from std/node as part of denoland#17840
andrewnester added a commit to andrewnester/deno that referenced this issue Mar 28, 2023
This PR ports string_decoder_test.ts from std/node as part of denoland#17840
bartlomieju added a commit that referenced this issue May 29, 2023
Part of #17840

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

---------

Co-authored-by: Bartek Iwańczuk <[email protected]>
@fbaltor
Copy link
Contributor

fbaltor commented Jun 10, 2023

Is this issue still available? I would like to port the remaining tests.

@ktfth
Copy link
Contributor

ktfth commented Jun 12, 2023

@kt3k The following PR are done for review:

#19461

Thank you so much

@ktfth
Copy link
Contributor

ktfth commented Jun 14, 2023

@kt3k The following PR are done for review:

#19505

Thank you in advice

@fbaltor
Copy link
Contributor

fbaltor commented Jun 14, 2023

Hey, I would like to tackle the node/_fs/_fs_writeFile_test.ts port. Just stating here so we avoid duplicate work.

@fbaltor
Copy link
Contributor

fbaltor commented Jun 22, 2023

The node/buffer_test.ts is ready in this PR: #19556

@fbaltor
Copy link
Contributor

fbaltor commented Jun 22, 2023

Port of node/crypto_test.ts done in: #19561

ktfth added a commit to ktfth/deno that referenced this issue Jun 22, 2023
added tests
added additionals params
added imports

denoland#17840
@ktfth
Copy link
Contributor

ktfth commented Jun 22, 2023

Read from filesystem PR done to check, please see here: #19588

@ktfth
Copy link
Contributor

ktfth commented Jun 27, 2023

@kt3k can you help me to understand what is happening on the checks? For #19609

@fbaltor
Copy link
Contributor

fbaltor commented Jun 27, 2023

@ktfth I guess the checks stop at the first failed task. In the case, it is an unformatted file getting caught in the lint debug ubuntu-x86_64:
image

@fbaltor
Copy link
Contributor

fbaltor commented Jun 28, 2023

How can we port the internals modules? For example node/internal/fs/streams_test.ts. In the deno repo the entities being tested (createReadStream, createWriteStream, ReadStream and WriteStream) are defined in the file ext/node/polyfills/internal/fs/streams.mjs, but there is no NodeModulePolyfill entry for it in the ext/node/polyfill.rs. Thus I cannot import it in a test file e.g. in cli/tests/unit_node/internal/streams_test.ts.

@bartlomieju
Copy link
Member

@fbaltor I believe you should be able to access them if you use const require = createRequire(import.meta.url) (but I'm not 100% sure about that).

@Kalmaegi
Copy link
Contributor

Kalmaegi commented Jun 29, 2023

I want to complete the migration of the following files:

  • node/perf_hooks_test.ts
  • node/process_test.ts
  • node/_fs/_fs_read_test.ts
  • node/child_process_test.ts
  • node/_util/_util_callbackify_test.ts
  • node/https_test.ts

@ktfth
Copy link
Contributor

ktfth commented Jun 29, 2023

@weartist the _fs_read_test.ts are complete, you can choose another I guess

@ktfth
Copy link
Contributor

ktfth commented Jun 29, 2023

@kt3k current pr's open by me, are done

@fbaltor
Copy link
Contributor

fbaltor commented Jun 30, 2023

@weartist there is a PR of the https_test as well, but it is failing, any insights are welcome: #19555

@Kalmaegi
Copy link
Contributor

@weartist there is an PR of the https_test as well, but it is failing, any insights are welcome: #19555还有一个 PR https_test ,但它失败了,欢迎任何见解:#19555

haha mine also failed, And I found that the previous http_test also failed

@ktfth
Copy link
Contributor

ktfth commented Jul 1, 2023

@fbaltor and @weartist maybe the suggestion who wheve made in the PR mentioned previously above could fits well.

@bartlomieju
Copy link
Member

Gonna close this one as most of the previous cases have been ported and we're actually running a part of Node.js native tests too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted community help requested
Projects
None yet
Development

No branches or pull requests

7 participants