Skip to content
This repository has been archived by the owner on Mar 29, 2019. It is now read-only.

Deceiving library reliability #1

Closed
lifenautjoe opened this issue May 4, 2016 · 9 comments
Closed

Deceiving library reliability #1

lifenautjoe opened this issue May 4, 2016 · 9 comments

Comments

@lifenautjoe
Copy link

The module has a build status badge on green deceiving the public about the reliability of the package.
The only existing test :

describe('cp-cli', () => {
  it('should do anything', () => {
    expect(true).to.be.true();
  });
@screendriver
Copy link
Owner

Yes that's true. I know that because I have written this one. This has some reasons:

  1. this tool is excatly one day old. I needed that tool immediately for a private project and I had no time to write a useful test.
  2. when you look at my code the only thing that I call is yargs and fs-extra. So the only thing to test would be to test that these both libraries are working as expected. It is only a simple wrapper around fs-extra#copy().
  3. I don't have any idea how to test the argumenst that yargs is reading because yargs read these from the command line.

So if you have any idea how to write a unit test for that any pull request would be welcome.

@lifenautjoe
Copy link
Author

I was going to make the exact same library, hence I found this one.
For the fs-extra, I do not know chai but we should use spies to check that the correct method was called with the correct arguments. For yargs, I'll investigate.

@lifenautjoe
Copy link
Author

Sharing the knowledge: https://jstesting.jcoglan.com/cli.pdf

@screendriver
Copy link
Owner

Thank you for sharing the link 👍

But as I said: at the moment there is no meaningfulness to write tests. The only thing we would test is that yargs and fs-extra are working as expected 😉 You shouldn't write unit tests that only test external libraries.

@lifenautjoe
Copy link
Author

No.. we would test that the cli application indeed uses the first argument as source and the second one as target and that fse.copy is called with these arguments in the correct order.

If this seems too trivial to you then why not remove the build passing badge? It should be trivial for users too right?

@screendriver
Copy link
Owner

I removed cp-cli-test.js for you. This file should originally be a skeleton for future unit tests.

The build passing badge remains because a "build" consists of two steps:

  1. Code linting
  2. Unit testing

I will add the one unit test that you wishes later when I have more time for that.

@lifenautjoe
Copy link
Author

I'll write it, today, as soon as I'm off work.

@screendriver
Copy link
Owner

Fixed with ff36430

@lifenautjoe
Copy link
Author

Awesome.

screendriver added a commit that referenced this issue May 6, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants