Skip to content

Remove usedevelop from tox.ini #18

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

Merged
merged 1 commit into from
Feb 14, 2022

Conversation

V02460
Copy link
Contributor

@V02460 V02460 commented Jan 26, 2022

Removes usedevelop=true from tox.ini, which is no longer needed because #15 added the tests to the source distribution.

Also fixes errors running tox -e py repeatedly, which was expecting a setup.py previously.

@V02460 V02460 requested a review from a team as a code owner January 26, 2022 22:40
@anoadragon453
Copy link
Member

Note that Synapse's copy of this setting has a few more words:

# As of twisted 16.4, trial tries to import the tests as a package (previously
# it loaded the files explicitly), which means they need to be on the
# pythonpath. Our sdist doesn't include the 'tests' package, so normally it
# doesn't work within the tox virtualenv.
#
# As a workaround, we tell tox to do install with 'pip -e', which just
# creates a symlink to the project directory instead of unpacking the sdist.
#
# (An alternative to this would be to set PYTHONPATH to include the project
# directory. Note two problems with this:
#
#   - if you set it via `setenv`, then it is also set during the 'install'
#     phase, which inhibits unpacking the sdist, so the virtualenv isn't
#     useful for anything else without setting PYTHONPATH similarly.
#
#   - `synapse` is also loaded from PYTHONPATH so even if you only set
#     PYTHONPATH for the test phase, we're still running the tests against
#     the working copy rather than the contents of the sdist. So frankly
#     you might as well use -e in the first place.
#
# )
usedevelop=true

Also note that Synapse includes the tests package in its source distribution, but set usedevelop=true afterwards in matrix-org/synapse#2439.

@reivilibre
Copy link
Collaborator

reivilibre commented Jan 31, 2022

This would be inline with what happened over here: matrix-org/synapse-module-cookiecutter-template#21

It's a bit of a workaround for a Tox bug but most of us don't habitually use Tox so wouldn't notice the slowdown anyway.

(N.B. I'm not sure the comment from Synapse is the whole truth. The reason it works in this scenario is because the -m in python -m twisted.trial adds . to the PYTHONPATH. Running as purely trial doesn't work not in editable mode IIRC? Though we likely don't care as long as we know how to do what works.)

Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, lgtm, but needs sign-off, please

@DMRobertson
Copy link
Contributor

475fc80 is signed-off.

@DMRobertson
Copy link
Contributor

Not fully sure how to merge this with #19 though (sorry, not familiar with tox). Could you take a quick look @V02460?

Signed-off-by: Kai A. Hiller <V02460@gmail.com>
@V02460
Copy link
Contributor Author

V02460 commented Feb 11, 2022

Fixed the conflict, forgot you dislike rebases 🙈

@anoadragon453 anoadragon453 merged commit 33cd2f3 into matrix-org:main Feb 14, 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.

5 participants