More useful Makefile (#226)

* More useful Makefile

* WIP

* Support async code for Python 3.1*

* Add release target to Makefile

* Run flake8 and pylint from venv/ in pre-commit.sh

* Add async_timeout to venv_requirements.txt

* Update python-package.yml

.github/workflows/python-package.yml

@@ -8,6 +8,9 @@ on:
     branches: [ master ]
   pull_request:
 
+env:
+  ENV_GITHUB_ACTIONS: 'ENV_GITHUB_ACTIONS'
+
 jobs:
   build:
 
@@ -25,22 +28,16 @@ jobs:
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
-        if python --version 2>&1 | grep -q "Python 2"; then pip install mock rsa==4.0 libusb1==1.9.3; fi
-        python -m pip install flake8 pylint coveralls cryptography libusb1>=1.0.16 pycryptodome
-        if [ -f requirements.txt ]; then pip install -r requirements.txt; fi
-        pip install .
-        if python --version 2>&1 | grep -q "Python 3.7" || python --version 2>&1 | grep -q "Python 3.8" || python --version 2>&1 | grep -q "Python 3.9" || python --version 2>&1 | grep -q "Python 3.10"; then pip install aiofiles async_timeout; fi
-    - name: Lint with pylint and flake8
+        make venv
+    - name: Linting checks with pylint, flake8, and (soon) black
       run: |
-        if python --version 2>&1 | grep -q "Python 2" || python --version 2>&1 | grep -q "Python 3.5" || python --version 2>&1 | grep -q "Python 3.6"; then flake8 adb_shell/ --exclude="adb_shell/adb_device_async.py,adb_shell/transport/base_transport_async.py,adb_shell/transport/tcp_transport_async.py" && pylint --ignore="adb_device_async.py,base_transport_async.py,tcp_transport_async.py" adb_shell/; fi
-        if python --version 2>&1 | grep -q "Python 3.7" || python --version 2>&1 | grep -q "Python 3.8" || python --version 2>&1 | grep -q "Python 3.9" || python --version 2>&1 | grep -q "Python 3.10"; then flake8 adb_shell/ && pylint adb_shell/; fi
-    - name: Test with unittest
+        make lint-flake8 lint-pylint
+    - name: Test with pytest
       env:
         GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         COVERALLS_SERVICE_NAME: github
       run: |
-        if python --version 2>&1 | grep -q "Python 2" || python --version 2>&1 | grep -q "Python 3.5" || python --version 2>&1 | grep -q "Python 3.6" ; then for synctest in $(cd tests && ls test*.py | grep -v async); do python -m unittest discover -s tests/ -t . -p "$synctest" || exit 1; done; fi
-        if python --version 2>&1 | grep -q "Python 3.7" || python --version 2>&1 | grep -q "Python 3.8" || python --version 2>&1 | grep -q "Python 3.9" || python --version 2>&1 | grep -q "Python 3.10"; then coverage run --source adb_shell -m unittest discover -s tests/ -t . && coverage report -m && coveralls; fi
+        make coverage && coveralls
     - name: Upload wheel as a workflow artifact
       uses: actions/upload-artifact@v2
       with:

Makefile

@@ -1,36 +1,199 @@
-.PHONY: release
-release:
-	rm -rf dist
-	rm -rf build
-	scripts/git_tag.sh
-	python setup.py sdist bdist_wheel
-	twine upload dist/*
+#-------------------- ONLY MODIFY CODE IN THIS SECTION --------------------#
+PACKAGE_DIR := adb_shell
+TEST_DIR := tests
+DOCS_DIR := docs
 
-.PHONY: docs
-docs:
-	rm -rf docs/build/html
-	@cd docs && sphinx-apidoc -f -e -o source/ ../adb_shell/
-	@cd docs && make html && make html
+# Change to false if you don't want to use pytest
+USE_PYTEST := true
+
+# Change this to false if you don't want to run linting checks on the tests
+LINT_TEST_DIR := false
+#-------------------- DO NOT MODIFY CODE BELOW!!!!!!!! --------------------#
+
+export PATH := $(abspath venv)/bin:${PATH}
+
+# Whether to include "*_async.py" files
+INCLUDE_ASYNC = $(shell python --version | grep -q "Python 3.[7891]" && echo "true" || echo "false")
+
+# Async vs. Sync files
+PACKAGE_ASYNC_FILES = $(shell ls -m $(PACKAGE_DIR)/*_async.py 2>/dev/null)
+TEST_ASYNC_FILES = $(shell ls -m $(TEST_DIR)/*_async.py 2>/dev/null)
+TEST_SYNC_FILES = $(shell cd $(TEST_DIR) && ls test*.py | grep -v async)
+
+# Target prerequisites that may or may not exist
+VENV_REQUIREMENTS_TXT := $(wildcard venv_requirements.txt)
+SETUP_PY := $(wildcard setup.py)
+
+# A prerequisite for forcing targets to run
+FORCE:
+
+# Help!
+help:  ## Show this help menu
+	@echo "\n\033[1mUsage:\033[0m"; \
+	awk -F ':|##' '/^[^\t].+?:.*?##/ { printf "\033[36m  make %-20s\033[0m %s\n", $$1, $$NF }' $(MAKEFILE_LIST) | grep -v "make venv/\." | sort
+	@echo ""
+	@echo "NOTES:"
+	@echo "- The 'venv/.bin' target may fail because newer Python versions include the 'venv' package.  Follow the instructions to create the virtual environment manually."
+ifneq ("$(wildcard scripts/pre-commit.sh)", "")
+	@echo "- To install the git pre-commit hook:\n\n    scripts/pre-commit.sh\n"
+endif
+	@echo "- You may need to activate the virtual environment prior to running any Make commands:\n\n    source venv/bin/activate\n"
+
+
+# Virtual environment targets
+.PHONY: clean-venv
+clean-venv:  ## Remove the virtual environment
+	rm -rf venv
+
+venv: venv/.bin venv/.requirements venv/.setup .git/hooks/pre-commit  ## Create the virtual environment and install all necessary packages
+
+venv/.bin:  ## Create the virtual environment
+	if [ -z "$$ENV_GITHUB_ACTIONS" ]; then \
+	  echo -e "If this target fails, you can perform this action manually via:\n\n    make clean-venv && python3 -m venv venv && source venv/bin/activate && pip install -U setuptools && echo -e '*.*\n**/' > venv/.gitignore && touch venv/.bin\n\n"; \
+	  apt list -a --installed python3-venv 2>&1 | grep -q installed || (sudo apt update && sudo apt install python3-venv); \
+	  python3 -m venv venv; \
+	  pip install -U setuptools; \
+	fi
+	mkdir -p venv
+	echo '*.*\n**/' > venv/.gitignore
+	touch venv/.bin
+
+venv/.requirements: venv/.bin $(VENV_REQUIREMENTS_TXT)  ## Install the requirements from 'venv_requirements.txt' in the virtual environment
+ifneq ("$(wildcard venv_requirements.txt)", "")
+	pip install -U -r venv_requirements.txt
+endif
+	touch venv/.requirements
+
+# Install the package in the virtual environment
+venv/.setup: venv/.bin $(SETUP_PY)
+ifneq ("$(wildcard setup.py)", "")
+	pip install .
+endif
+	touch venv/.setup
+
+.PHONY: uninstall
+uninstall:
+	rm -f venv/.setup
 
-SYNCTESTS := $(shell cd tests && ls test*.py | grep -v async)
+.PHONY: install
+install: uninstall venv/.setup  ## Install the package in the virtual environment
 
+# Create the pre-commit hook
+.git/hooks/pre-commit:
+	./scripts/pre-commit.sh MAKE_PRECOMMIT_HOOK
+
+.PHONY: pre-commit
+pre-commit: .git/hooks/pre-commit  ## Create the pre-commit hook
+
+# Linting and code analysis
+.PHONY: black
+black: venv  ## Format the code using black
+	black --safe --line-length 120 --target-version py35 $(PACKAGE_DIR)
+	black --safe --line-length 120 --target-version py35 $(TEST_DIR)
+ifneq ("$(wildcard setup.py)", "")
+	black --safe --line-length 120 --target-version py35 setup.py
+endif
+
+.PHONY: lint-black
+lint-black: venv  ## Check that the code is formatted using black
+	black --check --line-length 120 --safe --target-version py35 $(PACKAGE_DIR)
+	black --check --line-length 120 --safe --target-version py35 $(TEST_DIR)
+ifneq ("$(wildcard setup.py)", "")
+	black --check --line-length 120 --safe --target-version py35 setup.py
+endif
+
+.PHONY: lint-flake8
+lint-flake8: venv  ## Check the code using flake8
+ifeq ($(INCLUDE_ASYNC), true)
+	flake8 $(PACKAGE_DIR)
+ifeq ($(LINT_TEST_DIR), true)
+	flake8 $(TEST_DIR)
+endif
+else
+	flake8 $(PACKAGE_DIR) --exclude="$(PACKAGE_ASYNC_FILES)"
+ifeq ($(LINT_TEST_DIR), true)
+	flake8 $(TEST_DIR) --exclude="$(TEST_ASYNC_FILES)"
+endif
+endif
+ifneq ("$(wildcard setup.py)", "")
+	flake8 setup.py
+endif
+
+.PHONY: lint-pylint
+lint-pylint: venv  ## Check the code using pylint
+ifeq ($(INCLUDE_ASYNC), true)
+	pylint $(PACKAGE_DIR)
+ifeq ($(LINT_TEST_DIR), true)
+	pylint $(TEST_DIR)
+endif
+else
+	pylint $(PACKAGE_DIR) --ignore="$(PACKAGE_ASYNC_FILES)"
+ifeq ($(LINT_TEST_DIR), true)
+	pylint $(TEST_DIR) --ignore="$(TEST_ASYNC_FILES)"
+endif
+endif
+ifneq ("$(wildcard setup.py)", "")
+	pylint setup.py
+endif
+
+.PHONY: lint
+lint: lint-black lint-flake8 lint-pylint  ## Run all linting checks on the code
+
+
+# Testing and coverage.
 .PHONY: test
-test:
-	python --version 2>&1 | grep -q "Python 2" && (for synctest in $(SYNCTESTS); do python -m unittest discover -s tests/ -t . -p "$$synctest"; done) || true
-	python --version 2>&1 | grep -q "Python 3" && python -m unittest discover -s tests/ -t . || true
+test: venv  ## Run the unit tests
+ifeq ($(INCLUDE_ASYNC), true)
+ifeq ($(USE_PYTEST), true)
+	pytest $(TEST_DIR)
+else
+	python -m unittest discover -s $(TEST_DIR)/ -t .
+endif
+else
+ifeq ($(USE_PYTEST), true)
+	pytest $(TEST_DIR) --ignore-glob="*async.py"
+else
+	for synctest in $(TEST_SYNC_FILES); do echo "\033[1;32m$(TEST_DIR)/$$synctest\033[0m" && python -m unittest "$(TEST_DIR)/$$synctest"; done
+endif
+endif
 
 .PHONY: coverage
-coverage:
-	coverage run --source adb_shell -m unittest discover -s tests/ -t . && coverage html && coverage report -m
+coverage: venv  ## Run the unit tests and produce coverage info
+ifeq ($(INCLUDE_ASYNC), true)
+ifeq ($(USE_PYTEST), true)
+	coverage run --source $(PACKAGE_DIR) -m pytest $(TEST_DIR)/ && coverage report -m
+else
+	coverage run --source $(PACKAGE_DIR) -m unittest discover -s $(TEST_DIR) -t . && coverage report -m
+endif
+else
+ifeq ($(USE_PYTEST), true)
+	coverage run --source $(PACKAGE_DIR) -m pytest $(TEST_DIR)/ --ignore-glob="*async.py" && coverage report -m
+else
+	for synctest in $(TEST_SYNC_FILES); do echo "\033[1;32m$(TEST_DIR)/$$synctest\033[0m" && coverage run --source $(PACKAGE_DIR) -m unittest "$(TEST_DIR)/$$synctest"; done
+	coverage report -m
+endif
+endif
 
-.PHONY: tdd
-tdd:
-	coverage run --source adb_shell -m unittest discover -s tests/ -t . && coverage report -m
+.PHONY: htmlcov
+htmlcov: coverage  ## Produce a coverage report
+	coverage html
+
+
+# Documentation
+.PHONY: docs
+docs: venv  ## Build the documentation
+	rm -rf $(DOCS_DIR)/build
+	@cd $(DOCS_DIR) && sphinx-apidoc -f -e -o source/ $(CURDIR)/$(PACKAGE_DIR)/
+	@cd $(DOCS_DIR) && make html && make html
+
+
+.PHONY: release
+release:  ## Make a release and upload it to pypi
+	rm -rf dist
+	scripts/git_tag.sh
+	python setup.py sdist bdist_wheel
+	twine upload dist/*
 
-.PHONY: lint
-lint:
-	python --version 2>&1 | grep -q "Python 2" && (flake8 adb_shell/ --exclude="adb_shell/adb_device_async.py,adb_shell/transport/base_transport_async.py,adb_shell/transport/tcp_transport_async.py" && pylint --ignore="adb_device_async.py,base_transport_async.py,tcp_transport_async.py" adb_shell/) || (flake8 adb_shell/ && pylint adb_shell/)
 
-.PHONY: alltests
-alltests:
-	flake8 adb_shell/ && pylint adb_shell/ && coverage run --source adb_shell -m unittest discover -s tests/ -t . && coverage report -m
+.PHONY: all
+all: lint htmlcov  ## Run all linting checks and unit tests and produce a coverage report

scripts/pre-commit.sh

@@ -0,0 +1,71 @@
+#!/bin/bash
+
+set -e
+
+function make_pre_commit() {
+  # setup pre-commit hook
+  DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
+  echo -e "#!/bin/bash\n\n./scripts/pre-commit.sh 'placeholder_argument'" > "$DIR/../.git/hooks/pre-commit"
+  chmod a+x "$DIR/../.git/hooks/pre-commit"
+  echo "pre-commit hook successfully configured"
+}
+
+# if no arguments are passed, create the pre-commit hook
+if [ "$#" -eq 0 ]; then
+  read -p "Do you want to setup the git pre-commit hook? [Y/n]  " -n 1 -r
+  echo
+  if [[ $REPLY =~ ^[Yy]$ ]]; then
+    make_pre_commit
+  else
+    echo "pre-commit hook not configured"
+  fi
+  exit 0
+fi
+
+# if the argument passed is "MAKE_PRECOMMIT_HOOK", then make the pre-commit hook
+if [[ $1 == "MAKE_PRECOMMIT_HOOK" ]]; then
+  make_pre_commit
+  exit 0
+fi
+
+# THE PRE-COMMIT HOOK
+
+# get the directory of this script
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
+
+(
+  cd "$DIR/.."
+
+  no_unstaged_changes=true
+  echo -e "\n\033[1m1. Checking for unstaged changes...\033[0m"
+  for staged_file in $(git diff --name-only --cached); do
+    git diff --name-only | grep -q "${staged_file}" && echo "You have unstaged changes in '${staged_file}'" && no_unstaged_changes=false || true
+  done
+
+  # modified .py files
+  pyfiles=$(git diff --cached --name-only -- '*.py')
+
+  # flake8
+  flake8_pass=true
+  if [ "$pyfiles" != "" ]; then
+    echo -e "\n\033[1m2. Running flake8...\033[0m"
+    venv/bin/flake8 $pyfiles || flake8_pass=false
+  else
+    echo -e "\n\033[1m2. Skipping flake8.\033[0m"
+  fi
+
+  # pylint
+  pylint_pass=true
+  if [ "$pyfiles" != "" ]; then
+    echo -e "\n\033[1m3. Running pylint...\033[0m"
+    venv/bin/pylint $pyfiles || pylint_pass=false
+  else
+    echo -e "\n\033[1m3. Skipping pylint.\033[0m\n"
+  fi
+
+  if [ "$flake8_pass" != "true" ] || [ "$pylint_pass" != "true" ] || [ "$no_unstaged_changes" != "true" ]; then
+    echo -e "\033[1m\033[31mSome checks failed.\033[0m\n\n  NOT RECOMMENDED: If you want to skip the pre-commit hook, use the --no-verify flag.\n"
+    exit 1
+  fi
+  echo -e "\033[1m\033[32mAll checks passed.\033[0m\n"
+)

setup.py

@@ -1,3 +1,5 @@
+"""setup.py file for the adb_shell package."""
+
 from setuptools import setup
 
 with open('README.rst') as f:
@@ -15,7 +17,7 @@
     packages=['adb_shell', 'adb_shell.auth', 'adb_shell.transport'],
     install_requires=['cryptography', 'pyasn1', 'rsa'],
     tests_require=['pycryptodome', 'libusb1>=1.0.16'],
-    extras_require = {'usb': ['libusb1>=1.0.16'], 'async': ['aiofiles>=0.4.0', 'async_timeout>=3.0.0']},
+    extras_require={'usb': ['libusb1>=1.0.16'], 'async': ['aiofiles>=0.4.0', 'async_timeout>=3.0.0']},
     classifiers=['Operating System :: OS Independent',
                  'License :: OSI Approved :: Apache Software License',
                  'Programming Language :: Python :: 3',

tests/test_usb_importerror.py

@@ -5,7 +5,10 @@
 except ImportError:
     from mock import patch
 
-from adb_shell.transport.usb_transport import UsbTransport
+try:
+    from adb_shell.transport.usb_transport import UsbTransport
+except (ImportError, OSError):
+    UsbTransport = None
 
 
 class TestUsbImportError(unittest.TestCase):

tests/test_usb_transport.py

@@ -1,15 +1,19 @@
+"""Tests for the `UsbTransport` class."""
+
 import unittest
 
-try:
-    from unittest.mock import patch
-except ImportError:
-    from mock import patch
+from adb_shell.exceptions import TcpTimeoutException
 
-from adb_shell.transport.usb_transport import UsbTransport
+try:
+    from adb_shell.transport.usb_transport import UsbTransport
+except (ImportError, OSError):
+    UsbTransport = None
 
 from . import patchers
 
 
+# pylint: disable=missing-class-docstring, missing-function-docstring
+@unittest.skipIf(UsbTransport is None, "UsbTransport could not be imported")
 class TestUsbTransport(unittest.TestCase):
     def setUp(self):
         """Create a ``UsbTransport`` and do something...
@@ -19,7 +23,7 @@ def setUp(self):
 
         if True:
             return
-            
+
         with patchers.PATCH_CREATE_CONNECTION:
             self.transport.connect()