Skip to content
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

do not import mock, include tests in tarball #8

Closed
wants to merge 2 commits into from
Closed

do not import mock, include tests in tarball #8

wants to merge 2 commits into from

Conversation

pgajdos
Copy link
Contributor

@pgajdos pgajdos commented May 16, 2022

Fixes #7. Not completely sure about MANIFEST.in, I have little experience here.

@dkarchmer
Copy link
Owner

Can you explain why you need this change? What do you need tests to be part of the installation package?
I am not sure changing MANIFEST.in is enough as I think you also need to change setup.py but you are forcing everybody to get the tests for no clear reason. Is this a common thing for packages to do?

@pgajdos
Copy link
Contributor Author

pgajdos commented May 17, 2022

Look here:
https://build.opensuse.org/project/show/devel:languages:python
on *.spec files in individual packages, search for %check section in each spec file. There is a testsuite call in most cases. We ensure so that the package is working to some extent any time it is built.
In some git repositories, tags are created and that would be enough for us, as tag tarballs would contain tests (no need to add them to release tarballs IMO).

@dkarchmer
Copy link
Owner

But still trying to understand the goal. You want to be able to re-test the package after you install it (after you do pip install). I guess the only reason to do this would be to double check what the package builders should do.

I do accept that I have not had time to add testing to the CI process, so it depends on me testing before I release, but I did not know people want to keep retesting a package that was publicly released. I for sure don't want to be re-testing Django myself, and trust that the Django team tested the package before releasing.

Anyway, I don't think this change does anything other than removing mock so if you wanted to add testing to the package, you have not done so. I will investigate at some point and see if I can find a good example for a simple package I can borrow from

@dkarchmer
Copy link
Owner

From a quick scan, I don't see djangorestframework or djangorestfrmework-simplejwt (https://github.com/jazzband/djangorestframework-simplejwt) including tests in the installation. In fact, they very explicitly exclude them.

Sorry, but I don't think I am going to accept a PR for this until I am pointed to a GitHub repo for a simple package that does this

@pgajdos
Copy link
Contributor Author

pgajdos commented May 18, 2022

Yep, as I have written in my first comment, I am not sure about the part that includes tests and I do not know how to test. Let's close this.

@pgajdos pgajdos closed this May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests in pypi tarball + unittest.mock
2 participants