LibraryOfCongress / bagit-python

Work with BagIt packages from Python.
http://libraryofcongress.github.io/bagit-python
216 stars 85 forks source link

Add tests to cover the CLI #161

Closed nkrabben closed 1 year ago

nkrabben commented 2 years ago

A couple years ago I wanted to add CLI tests.

@acdha is it okay if I add tests to cover the CLI? It would have caught the bug I fixed in 61769cd, but I hadn't seen an approach to testing in the package yet.

Definitely – that's a big gap in the test coverage currently

Originally posted by @acdha in https://github.com/LibraryOfCongress/bagit-python/issues/138#issuecomment-585262171

I've since tried a couple strategies on other projects, none of which are clearly preferably. The two strategies I'm consider are:

  1. Unit test _make_parser() which is relatively straightforward but leaves main() untested, where bugs are more likely to occur.
  2. Function test main() which is more complete but repeats many of the existing tests, just with an additional abstraction layer.

My preference would be to test main() against the following cases.

  1. Not enough processes
  2. Invalid use of --fast
  3. Invalid use of --completeness-only
  4. Successful validate --fast
  5. Successful validate --completeness-only
  6. Successful validate
  7. Unsuccessful validate --fast
  8. Unsuccessful validate --completeness-only
  9. Unsuccessful validate
  10. Successful bag creation
  11. Unsuccessful bag creation

Does that sound like a good strategy? I'm happy to go in another direction.

kba commented 1 year ago

My preference would be to test main() against the following cases.

  1. Not enough processes
  2. Invalid use of --fast
  3. Invalid use of --completeness-only
  4. Successful validate --fast
  5. Successful validate --completeness-only
  6. Successful validate
  7. Unsuccessful validate --fast
  8. Unsuccessful validate --completeness-only
  9. Unsuccessful validate
  10. Successful bag creation
  11. Unsuccessful bag creation

That sound like a good and thorough approach to me :+1: