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

WIP: Refactor nrm #114

Merged
merged 35 commits into from
Nov 18, 2021
Merged

WIP: Refactor nrm #114

merged 35 commits into from
Nov 18, 2021

Conversation

CaptainOfPhB
Copy link
Contributor

@CaptainOfPhB CaptainOfPhB commented Nov 8, 2021

This is a Work In Progress merge request, CAN NOT merge directly now.

Motivation

When I tried to add some test cases for cli.js, I found some test cases will run fail for some reason, for example, npm.config.get('registry') will get a wrong registry(https://registry.yarnpkg.com/) sometimes, I am sure about that the registry(https://registry.yarnpkg.com/) does not exist in any related configuration file.

In addition, use npm lib to write .npmrcfile is unnecessary, fs.writeFileSync is enough. And the programmatic API was removed innpm from v8.0.0, as we can see in index.js, so, the var npm = require('npm') usage is not recommended and already has been deprecated.

I will do

  • remove npm dependency
  • replace request with node-fetch
  • split project files and make the code more concise
  • add more test cases

@CaptainOfPhB
Copy link
Contributor Author

CaptainOfPhB commented Nov 17, 2021

please review code @i5ting @Pana

@i5ting
Copy link
Collaborator

i5ting commented Nov 18, 2021

1、tap测试是没有问题
2、test-console用起来没有coffee用的直观
3、可以把sinon引入,理解一下spies,stubs,mocks,测试用例代码看着可读性还需要提升。

@CaptainOfPhB
Copy link
Contributor Author

CaptainOfPhB commented Nov 18, 2021

  1. test-console 可以测试单个函数的输出,看了下 coffee,类似于 child_process,直接从命令行作为入口,测试输出,这和测试单个函数并无明显区别。
  2. tap 已具备 mock 功能,虽然没有 sinon 断言 spy 函数被调用那样便捷的 api,但是通过 test-console 测试 mock 函数的输出也可以达到同样的效果,即 mock 的函数被调用了。

@i5ting i5ting merged commit 1fd3f2f into Pana:master Nov 18, 2021
@i5ting
Copy link
Collaborator

i5ting commented Nov 18, 2021

功能是类似的,可读性上是有差异的。另外我理解coffee对跨平台做了很多兼容。

tap的mock功能我还没看到,https://github.com/tapjs/node-tap/blob/main/package.json#L22依赖有点多。

先合并,一点点优化吧

@CaptainOfPhB
Copy link
Contributor Author

OK,谢狼叔指点。

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.

2 participants