When looking at the test, it seems that there's a problem with the spo file add command test coverage.

Didn't see that until when submitting my PR, so I've looked back locally and saw this when running a global npm run test:

Am I missing something?

Which version of Node are you using?

v10.16.0

... And I just saw this.

Ok sorry, my bad.

nvm to the rescue 😬

I can repro on Node@14 3 failing tests in spo file add. Let's create a separate issue for that. Thanks for reporting 👏

Hey @michaelmaillot did you have a chance to look at my comments? 🙂

Yes!

And everything is almost ready for the commit. I'm just stuck with a few tests on the Cli.ts to cover the init of the Configstore property if doesn't exist...

Awesome! Looking forward to it 👏

So as you can see, not all the Cli.ts file is covered. The fact is that I didn't find how to "assert" that the Cli.conf property is set if null (you can see that I instantiate the property in the test file). I'm probably missing something about understanding how sinon.js works.

Sorry to ask but I need help here, I'm stuck.

And if you have global guidance about test coverage, beyond what exists in the repo, I'll appreciate it.

Thanks.

Edit: I didn't notice that this command is awaited for more updates 😅 maybe I could take the new requests once this command is available (if you don't mind and if no one's on it of course)

Appreciate your patience and effort to get this in. I'll have a look asap and we'll get it fixed together.

When writing tests and having unusual test cases, I try to find either something similar in the CLI codebase or looking on the internet for some specific examples. Sorry I don't have anything more specific for you.

Thank you for your patience.

I'm conforted in my approach, because that's also how I usually work. But in that case, when I'm close to a solution, it sounds inappropriate / not convenient to my problem.

Here for example, I tried to "test" if my object was instantiated and solution found was to make a constructor for the test, which sounds "huge" for my needs.

But maybe I'm wrong.

Edit: I'm not used to call out for help. It's more the opposite: I try (sometimes for too long) to find by myself. But this time, I really have the feeling that I'm in a dead end (I can be wrong in my current testing approach)

Nothing wrong with asking for help and I appreciate you doing your best to get things to the best possible shape. Let's check out the code and see where we stand. 👍

Short update: I've refactored the code and I'm on 100% coverage now. Last thing left is to review docs and we'll be ready to get this in 👏 I'll submit a review with my findings and suggestions when ready

Merged manually. Thank you! 👏

Thank you for taking time refractoring my code, didn't think about using a getter to centralize the check. Sounds way better like that.

I've added some comments, when you got a couple of minutes to have a look....

Happy to help and you've done all the hard work getting the structure in. Thank you for your help 👏