Pana / nrm

NPM registry manager, fast switch between different registries: npm, cnpm, nj, taobao
MIT License
2.79k stars 244 forks source link

WIP: Refactor nrm #114

Closed CaptainOfPhB closed 2 years ago

CaptainOfPhB commented 2 years ago

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

CaptainOfPhB commented 2 years ago

please review code @i5ting @Pana

i5ting commented 2 years ago

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

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

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

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

先合并,一点点优化吧

CaptainOfPhB commented 2 years ago

OK,谢狼叔指点。