kislyuk / yq

Command-line YAML, XML, TOML processor - jq wrapper for YAML/XML/TOML documents
https://kislyuk.github.io/yq/
Apache License 2.0
2.62k stars 84 forks source link

Include tests in PyPI tarball #53

Closed dotlambda closed 5 years ago

dotlambda commented 5 years ago

This is useful for downstream distributions to test if the package works correctly with their build recipe. Also, it provides a way to test if the dependencies as they are packaged by the distribution are compatible.

codecov-io commented 5 years ago

Codecov Report

Merging #53 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #53   +/-   ##
=======================================
  Coverage   85.31%   85.31%           
=======================================
  Files           2        2           
  Lines         177      177           
=======================================
  Hits          151      151           
  Misses         26       26

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8de4670...ece0aea. Read the comment docs.

kislyuk commented 5 years ago

I don't think the package should distribute either test or tests as a top level module name.

Can you point me to a document describing the best practice for distributing tests with the source via PyPI? I did find one answer that references the idea that tests should be included inside the package (e.g. in yq/tests) and I think that's a bad approach, too, since tests should be separate from the package itself.

dotlambda commented 5 years ago

This will not install tests as a module. It will simply include the directory in an sdist.

kislyuk commented 5 years ago

You're right, this doesn't seem to result in a namespace collision.

The doc I was looking for is here: https://docs.python.org/3.7/distutils/sourcedist.html

Your PR is a no-op because the test/test.py file is already included by default. The correct line is simply recursive-include test *.

kislyuk commented 5 years ago

Fixed in ee307fc.