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

Remove sys.path hack in run_tests to avoid issues with compiled code #3912

Merged
merged 2 commits into from
Apr 25, 2022

Conversation

JoaoRodrigues
Copy link
Member

  • I hereby agree to dual licence this and any previous contributions under both
    the Biopython License Agreement AND the BSD 3-Clause License.

  • I have read the CONTRIBUTING.rst file, have run pre-commit
    locally, and understand that continuous integration checks will be used to
    confirm the Biopython unit tests and style checks pass with these changes.

  • I have added my name to the alphabetical contributors listings in the files
    NEWS.rst and CONTRIB.rst as part of this pull request, am listed
    already, or do not wish to be listed. (This acknowledgement is optional.)

If the CI passes (this is the only way to test), this patch closes #3910.

@JoaoRodrigues JoaoRodrigues self-assigned this Apr 25, 2022
@JoaoRodrigues
Copy link
Member Author

JoaoRodrigues commented Apr 25, 2022

I don't understand why appveyor would fail, since all windows tests pass on GHA.. However, is the purpose of the Appveyor workflow to 1) test BioSQL and 2) upload the coverage results to codecov? These could be moved to GHA as well.

EDIT: python setup.py build instead of install. The removal of the code from run_tests.py breaks that.

@JoaoRodrigues JoaoRodrigues merged commit 644f041 into biopython:master Apr 25, 2022
@JoaoRodrigues JoaoRodrigues deleted the remove_syspath_hacks branch April 26, 2022 15:28
@peterjc
Copy link
Member

peterjc commented Apr 27, 2022

That makes sense. So we can no longer do this on a clean machine:

python setup.py build
python setup.py test
python setup.py install

Being able to test before install is helpful, but not essential given how most people now install pre-compiled wheels or other packages.

npars added a commit to npars/biopython that referenced this pull request May 20, 2022
- PR biopython#3912 introduced changes that broke the existing testing
  instructions
- Update testing instructions to work with latest test scripts
- Test instructions now work with setup.py script as well as through the
  Intellij test runner
JoaoRodrigues added a commit to cbalbin-bio/biopython that referenced this pull request Aug 7, 2022
…ness (biopython#3912)

- Remove sys.path hacking from `run_tests`
- Update CI workflow to use `pip` instead of `python setup.py install`
- Force-update build dependencies (setuptools, pip, wheel) across all 3 OSes.
peterjc pushed a commit that referenced this pull request Oct 21, 2022
- PR #3912 introduced changes that broke the existing testing
  instructions
- Update testing instructions to work with latest test scripts
- Test instructions now work with setup.py script as well as through the
  Intellij test runner
peterjc added a commit to peterjc/biopython that referenced this pull request Apr 6, 2023
Should have done this around the time of biopython#3937,
old method broke as a result of biopython#3912
peterjc added a commit that referenced this pull request Apr 23, 2023
Should have done this around the time of #3937,
old method broke as a result of #3912
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.

CI fails because of import errors with C extensions
2 participants