E532 Include tests in source distributions by sbraz · Pull Request #1976 · pydantic/pydantic · GitHub
[go: up one dir, main page]
Skip to content

Include tests in source distributions #1976

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.

Sign up for GitHub

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

Jump to bottom
Merged
samuelcolvin merged 1 commit into from
Oct 9, 2020
Merged

Include tests in source distributions #1976

samuelcolvin merged 1 commit into from
Oct 9, 2020

Conversation

@sbraz
Copy link
Contributor
@sbraz sbraz commented Oct 8, 2020

Hi,
We at Gentoo (and Debian too AFAIK) tend to use PyPI tarballs to run tests. Could you please include them in the next release?

Change Summary

Include tests in source distributions.

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)

@codecov
Copy link
codecov bot commented Oct 8, 2020 •
edited
Loading

Codecov Report

Merging #1976 into master will decrease coverage by 0.15%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##            master    #1976      +/-   ##
===========================================
- Coverage   100.00%   99.84%   -0.16%     
===========================================
  Files           21       42      +21     
  Lines         3909     7818    +3909     
  Branches       788     1576     +788     
===========================================
+ Hits          3909     7806    +3897     
- Misses           0       10      +10     
- Partials         0        2       +2
Impacted Files Coverage Δ
D:/a/pydantic/pydantic/pydantic/fields.py 100.00% <0.00%> (ø)
D:/a/pydantic/pydantic/pydantic/utils.py 99.21% <0.00%> (ø)
D:/a/pydantic/pydantic/pydantic/json.py 100.00% <0.00%> (ø)
D:/a/pydantic/pydantic/pydantic/validators.py 100.00% <0.00%> (ø)
D:/a/pydantic/pydantic/pydantic/datetime_parse.py 100.00% <0.00%> (ø)
D:/a/pydantic/pydantic/pydantic/env_settings.py 97.43% <0.00%> (ø)
D:/a/pydantic/pydantic/pydantic/types.py 99.57% <0.00%> (ø)
D:/a/pydantic/pydantic/pydantic/mypy.py 100.00% <0.00%> (ø)
D:/a/pydantic/pydantic/pydantic/decorator.py 100.00% <0.00%> (ø)
D:/a/pydantic/pydantic/pydantic/error_wrappers.py 100.00% <0.00%> (ø)
... and 11 more

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 bf9cc4a...08af5c0. Read the comment docs.

@sbraz
Copy link
Contributor Author
sbraz commented Oct 8, 2020

I don't really understand how I could have changed the coverage, is that a bug?

@samuelcolvin
Copy link
Member
samuelcolvin commented Oct 9, 2020 •
edited
Loading

The coverage issue is something to do with codecov, it's using a different directory for windows tests and use showing lines missed in windows as misses, look in coverage. I'll try and fix it.

On the actual issue, I don't really wan to include tests in the .whl files if I can avoid it. Is there any way to include them in .tar.gz but not in .whl? Or is that already implemented somehow.

Have you build the binaries and .tar.gzand checked this is working? See 55d1305 for an example of how to do this easily. scrap that, you'll need to build them locally to check.

@samuelcolvin samuelcolvin mentioned this pull request Oct 9, 2020
Closed
@sbraz
Copy link
Contributor Author
sbraz commented Oct 9, 2020

On the actual issue, I don't really wan to include tests in the .whl files if I can avoid it. Is there any way to include them in .tar.gz but not in .whl? Or is that already implemented somehow.

I ran bdist_wheel and the test directory isn't included. It looks like wheel doesn't really care about MANIFEST.in.

Have you build the binaries and .tar.gzand checked this is working?

Yes, the generated sdist does contain the test directory. This syntax in MANIFEST.in is more or less boilerplate that I add every times tests aren't included.

@samuelcolvin
Copy link
Member
samuelcolvin commented Oct 9, 2020

looks good to me.

My only question would be around whether requirements from tests/requirements.txt are required somehow. Do we need to set tests_require in setup.py?

@sbraz
Copy link
Contributor Author
sbraz commented Oct 9, 2020

I think tests_require are only significant if you run setup.py test and I doubt many people still do that nowadays.

The file does get added to the sdist (at least on my setup) but I don't think it's a problem. It won't get installed anyway so it won't pollute the system.

@samuelcolvin
Copy link
Member
samuelcolvin commented Oct 9, 2020

The file does get added to the sdist (at least on my setup) but I don't think it's a problem. It won't get installed anyway so it won't pollute the system.

I realise that, but I thought if you wanted your platform to run tests, you might need a way to define requirements for those tests, hence suggesting tests_require. As per the following line, all that's actually required to run tests is pytest and pytest-mock

https://github.com/samuelcolvin/pydantic/blob/078e7090d01d81cbcb4dbc662624bd9a33b38a43/.github/workflows/ci.yml#L227

If you're happy with this I'll merge it.

@samuelcolvin samuelcolvin merged commit e8326f8 into pydantic:master Oct 9, 2020
@samuelcolvin
Copy link
Member
samuelcolvin commented Oct 9, 2020

thanks

@sbraz
Copy link
Contributor Author
sbraz commented Oct 9, 2020

I don't think pytest-mock is even required, tests pass without it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sbraz @samuelcolvin
0