-
Notifications
You must be signed in to change notification settings - Fork 85
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
Convert tests to unittest #28
Convert tests to unittest #28
Conversation
97e885d
to
afaceff
Compare
I just realized a MANIFEST.in is required to include the test data into the distribution tarball ("python setup.py sdist"). That means that moving "tests" to "test" was unnecessary. I will therefore revert the first commit and instead add the test source code to the MANIFEST |
4d8864f
to
ddf8397
Compare
Codecov Report
@@ Coverage Diff @@
## master #28 +/- ##
===========================================
+ Coverage 36.68% 86.72% +50.04%
===========================================
Files 13 13
Lines 1145 1145
===========================================
+ Hits 420 993 +573
+ Misses 725 152 -573
Continue to review full report at Codecov.
|
Amazing! Thank you so much. While the library usage of msoffcrpyo-tool is now nicely covered by your new tests, I intended to test the CLI usage of the tool using That is, ideally there should be:
So, my suggestion is to merge your PR preserving |
Glad you like it :-) |
Required e.g. for tarballs created using python setup.py sdist (e.g. for pip) to include tests and the readme file
This way can run nosetests before build and install
9b17081
to
f20cfad
Compare
Merged! I'll release a new version with this update soon. |
Cool :-) Thanks |
Sofar, there was a simple tests.sh script for testing, that required msoffcrypto-tool was already installed.
Converted that script to a python unittest that performs the same action but can run without installing msoffcrypto-tool
Renamed dir tests to test because when building rpms/source tarballs using setup.py, "test" is the hard-coded name where tests are expected. Should be included that way