Update makefile to create virtual environment for running tools #354
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary:
It is an antipattern to require contributors to install tools on their stock python and/or to manually manage virtual environments.
This change ensures that Makefile runs do not expect anything present on the system beyond "stock" python
Running on an host where previously expected tools are not installed
```
maci:testslide/ (maci-venv-support) $ which python
python not found
maci:testslide/ (maci-venv-support) $ which pip
pip not found
maci:testslide/ (maci-venv-support) $ which virtualenv
virtualenv not found
maci:testslide/ (maci-venv-support) $
```
### `make clean`
```
maci:testslide/ (maci-venv-support) $ git clean -fdx
maci:testslide/ (maci-venv-support) $ make clean
SDIST CLEAN
DOCS CLEAN
COVERAGE HTML CLEAN
COVERAGE ERASE
MYPY CLEAN
CLEAN
maci:testslide/ (maci-venv-support) $ git clean -fdx
maci:testslide/ (maci-venv-support) $
```
### `make docs`
```
maci:testslide/ (maci-venv-support) $ git clean -fdx
maci:testslide/ (maci-venv-support) $ make docs
CREATE VIRTUALENV (/Users/maci/github/testslide/venv)
INSTALL BUILD DEPS
INSTALL DEPS
DOCS
maci:testslide/ (maci-venv-support) $ git clean -fdx -n
Would remove docs/_build/
Would remove venv/
maci:testslide/ (maci-venv-support) $
```
### `make format`
```
maci:testslide/ (maci-venv-support) $ git clean -fdx
maci:testslide/ (maci-venv-support) $ make format
CREATE VIRTUALENV (/Users/maci/github/testslide/venv)
INSTALL BUILD DEPS
INSTALL DEPS
FORMAT PYFMT tests testslide util pytest-testslide
FORMAT BLACK tests testslide util pytest-testslide
maci:testslide/ (maci-venv-support) $ git clean -fdx -n
Would remove venv/
maci:testslide/ (maci-venv-support) $
```
### `make tests` and `make coverage_report` work (NOTE: venv is not being recreated as exists already)
```
maci:testslide/ (maci-venv-support) $ git clean -fdx
maci:testslide/ (maci-venv-support) $ make tests
CREATE VIRTUALENV (/Users/maci/github/testslide/venv)
INSTALL BUILD DEPS
INSTALL DEPS
COVERAGE ERASE
UNITTEST tests/accept_any_arg_unittest.py
UNITTEST tests/cli_unittest.py
UNITTEST tests/dsl_unittest.py
UNITTEST tests/matchers_unittest.py
UNITTEST tests/pep487_unittest.py
UNITTEST tests/testcase_unittest.py
TESTSLIDE tests/lib_testslide.py
TESTSLIDE tests/mock_async_callable_testslide.py
TESTSLIDE tests/mock_callable_testslide.py
TESTSLIDE tests/mock_constructor_testslide.py
TESTSLIDE tests/patch_attribute_testslide.py
TESTSLIDE tests/strict_mock_testslide.py
INSTALL pytest_testslide DEPS
PYTEST pytest_testslide
MYPY tests testslide util pytest-testslide
FLAKE8 tests testslide util pytest-testslide
ISORT tests testslide util pytest-testslide
BLACK tests testslide util pytest-testslide
Copyright structure intact for all python files
maci:testslide/ (maci-venv-support) $ make coverage_report
COVERAGE ERASE
UNITTEST tests/accept_any_arg_unittest.py
UNITTEST tests/cli_unittest.py
UNITTEST tests/dsl_unittest.py
UNITTEST tests/matchers_unittest.py
UNITTEST tests/pep487_unittest.py
UNITTEST tests/testcase_unittest.py
TESTSLIDE tests/lib_testslide.py
TESTSLIDE tests/mock_async_callable_testslide.py
TESTSLIDE tests/mock_callable_testslide.py
TESTSLIDE tests/mock_constructor_testslide.py
TESTSLIDE tests/patch_attribute_testslide.py
TESTSLIDE tests/strict_mock_testslide.py
COVERAGE COMBINE
COVERAGE REPORT
maci:testslide/ (maci-venv-support) $
```
### `make sdist` works
```
maci:testslide/ (maci-venv-support) $ git clean -fdx
maci:testslide/ (maci-venv-support) $ make sdist
SDIST
maci:testslide/ (maci-venv-support) $ git clean -fdx -n
Would remove TestSlide.egg-info/
Would remove dist/
maci:testslide/ (maci-venv-support) $
```
### `make ci` works
```
maci:testslide/ (maci-venv-support) $ git clean -fdx
maci:testslide/ (maci-venv-support) $ make ci
CREATE VIRTUALENV (/Users/maci/github/testslide/venv)
INSTALL BUILD DEPS
INSTALL DEPS
COVERAGE ERASE
UNITTEST tests/accept_any_arg_unittest.py
UNITTEST tests/cli_unittest.py
UNITTEST tests/dsl_unittest.py
UNITTEST tests/matchers_unittest.py
UNITTEST tests/pep487_unittest.py
UNITTEST tests/testcase_unittest.py
TESTSLIDE tests/lib_testslide.py
TESTSLIDE tests/mock_async_callable_testslide.py
TESTSLIDE tests/mock_callable_testslide.py
TESTSLIDE tests/mock_constructor_testslide.py
TESTSLIDE tests/patch_attribute_testslide.py
TESTSLIDE tests/strict_mock_testslide.py
INSTALL pytest_testslide DEPS
PYTEST pytest_testslide
MYPY tests testslide util pytest-testslide
FLAKE8 tests testslide util pytest-testslide
ISORT tests testslide util pytest-testslide
BLACK tests testslide util pytest-testslide
Copyright structure intact for all python files
COVERAGE COMBINE
COVERAGE REPORT
DOCS
SDIST
INSTALL LOCAL
maci:testslide/ (maci-venv-support) $
```
**What:**
Ensure that `make {CMD}` work without expectations of developers setup.
**Why:**
Running tests, build documentation or anything from the Makefile requires developer to have some tools installed into their system and by default everything gets installed on the used interpreter.
This is an anti-pattern as python virtual environments are better suited to build developer environments.
**How:**
Ensuring that Makefile defines a `venv` target which ensures that a virtual environment is created with all the needed dependencies and it is used as dependency for all the other targets (ie. format, test, etc.)
**Risks:**
No risks are induced by this PR as it updates makefile and not library code and makefile changes have been verified in the test plan and cross-examined by github actions
**Checklist**:
- [ ] Added tests, if you've added code that should be tested
- [ ] Updated the documentation, if you've changed APIs
- [x] Ensured the test suite passes
- [x] Made sure your code lints
- [x] Completed the Contributor License Agreement ("CLA")
Reviewed By: deathowl
Differential Revision: D46856622
fbshipit-source-id: 8bbdaee9ee679cdac620719a0631f963c1b03a1b
facebook-github-bot
added
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D46856622 |
|
This PR is the "same" of #353 (exported from meta internal infra) |
Pull Request Test Coverage Report for Build 5322457985Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
|
This pull request has been merged in de9e2c1. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
It is an antipattern to require contributors to install tools on their stock python and/or to manually manage virtual environments.
This change ensures that Makefile runs do not expect anything present on the system beyond "stock" python
Running on an host where previously expected tools are not installed
make cleanmake docsmake formatmake testsandmake coverage_reportwork (NOTE: venv is not being recreated as exists already)make sdistworksmake ciworksWhat:
Ensure that
make {CMD}work without expectations of developers setup.Why:
Running tests, build documentation or anything from the Makefile requires developer to have some tools installed into their system and by default everything gets installed on the used interpreter.
This is an anti-pattern as python virtual environments are better suited to build developer environments.
How:
Ensuring that Makefile defines a
venvtarget which ensures that a virtual environment is created with all the needed dependencies and it is used as dependency for all the other targets (ie. format, test, etc.)Risks:
No risks are induced by this PR as it updates makefile and not library code and makefile changes have been verified in the test plan and cross-examined by github actions
Checklist:
Reviewed By: deathowl
Differential Revision: D46856622