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

Update pytest to latest version #330

Merged
merged 4 commits into from Jul 3, 2021
Merged

Conversation

ashb
Copy link
Contributor

@ashb ashb commented May 17, 2021

The tests/conftest.py is a bit of a hack, but needed to support running tests against a non-editable install (which is the recommended way as per https://docs.pytest.org/en/latest/explanation/goodpractices.html?highlight=tests%20outside#choosing-a-test-layout-import-rules )

Closes #159

@codecov
Copy link

codecov bot commented May 17, 2021

Codecov Report

Merging #330 (f08a38b) into master (b801118) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #330   +/-   ##
=======================================
  Coverage   70.95%   70.95%           
=======================================
  Files          80       80           
  Lines        2355     2355           
=======================================
  Hits         1671     1671           
  Misses        684      684           

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 b801118...f08a38b. Read the comment docs.

@p1c2u
Copy link
Collaborator

p1c2u commented May 19, 2021

Hi @ashb thanks for the PR. I have few questions:

a bit of a hack

How not tacky way would look like?

support running tests against a non-editable install

I'm curious, why you need to run these tests?

recommended way as per https://docs.pytest.org/en/latest/explanation/goodpractices.html?highlight=tests%20outside#choosing-a-test-layout-import-rules

Do you mean "Tests outside application code" section or can you point me to exact paragraph?

Thanks in advance.

@ashb
Copy link
Contributor Author

ashb commented May 20, 2021

@p1c2u I haven't run in to this issue before with other pytest projects, so I'm not sure why I've hit it here (perhaps I have always just run with pip install -e . && pytest)

So there are two non-hacky ways we could do this. (I define hacky as having to tweak sys.path)

  • Update docs and CI to do pip install -e . before running tests
  • Convert to the src/ and tests/ layout which seems to be what Pytest recommends.

I don't have a strong preference either way. Do you?

@uranusjr
Copy link

Another way to do this is to use pytest-pythonpath and set

[tool:pytest]
pythonpath = .

in setup.cfg.

But always installing the package is still the better idea.

I think a lot of project these days don't have that issue since they tend to use nox or tox to run tests instead (they automatically reinstall the package into the test environment before invoking the test suite).

@p1c2u
Copy link
Collaborator

p1c2u commented May 21, 2021

Thanks guys,

Python 2 support was dropped. Nothing stopping us now to update to pytest 6.

Convert to the src/ and tests/ layout which seems to be what Pytest recommends.

If so then we should go with the recommended way.

@ashb
Copy link
Contributor Author

ashb commented May 25, 2021

👍 I'll update this to use the recommended way.

@ashb
Copy link
Contributor Author

ashb commented Jun 24, 2021

@p1c2u Updated this PR.

I've left it with the same source layout, but added -e . in to requirements_dev.txt` so that it's installed that way.

setup.cfg Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
@p1c2u
Copy link
Collaborator

p1c2u commented Jul 3, 2021

Updated pytest with new poetry build system

@p1c2u
Copy link
Collaborator

p1c2u commented Jul 3, 2021

@ashb @uranusjr thank you for the contribution

@p1c2u p1c2u merged commit b65c972 into python-openapi:master Jul 3, 2021
@ashb ashb deleted the upgrade-pytest branch July 3, 2021 13:36
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.

None yet

3 participants