From b95acea973c20eea3e7cbbca42d09b1f5d4a3412 Mon Sep 17 00:00:00 2001 From: Jamie Hewland Date: Sun, 6 Sep 2020 12:08:07 +0200 Subject: [PATCH] Update CI scripts to match httpcore (#1043) * Update CI scripts to match httpcore * Run test suite on pushes to master * Update scripts README * Don't bother with flake8 extensions for now * Remove unnecessary PYTHONPATH from build, publish * test_routing: Use a stub app instead of ellipsis * Add link to issue about type-checking tests --- .github/workflows/publish.yml | 4 +++- .github/workflows/test-suite.yml | 6 ++++++ .gitignore | 10 ++++++++-- requirements.txt | 5 +++++ scripts/README.md | 5 ++++- scripts/build | 13 +++++++++++++ scripts/check | 14 ++++++++++++++ scripts/coverage | 10 ++++++++++ scripts/docs | 10 ++++++++++ scripts/lint | 9 ++++----- scripts/publish | 3 --- scripts/test | 19 ++++++++++--------- setup.cfg | 19 +++++++++++++++++++ starlette/applications.py | 5 +++-- starlette/config.py | 6 ++++-- starlette/graphql.py | 2 +- starlette/middleware/cors.py | 4 ++-- starlette/middleware/errors.py | 13 +++++++++---- starlette/middleware/wsgi.py | 3 ++- starlette/requests.py | 3 ++- starlette/routing.py | 6 +++--- starlette/staticfiles.py | 3 ++- tests/middleware/test_cors.py | 2 +- tests/test_authentication.py | 4 ++-- tests/test_formparsers.py | 22 ++++++++++++++++------ tests/test_requests.py | 6 +++--- tests/test_routing.py | 6 +++++- 27 files changed, 161 insertions(+), 51 deletions(-) create mode 100755 scripts/build create mode 100755 scripts/check create mode 100755 scripts/coverage create mode 100755 scripts/docs create mode 100644 setup.cfg diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index d8e007f10..a41fd2bf2 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -18,7 +18,9 @@ jobs: python-version: 3.7 - name: "Install dependencies" run: "scripts/install" - - name: "Publish" + - name: "Build package & docs" + run: "scripts/build" + - name: "Publish to PyPI & deploy docs" run: "scripts/publish" env: TWINE_USERNAME: __token__ diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index 60ec0f7ec..d7b105008 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -2,6 +2,8 @@ name: Test Suite on: + push: + branches: ["master"] pull_request: branches: ["master"] @@ -21,5 +23,9 @@ jobs: python-version: "${{ matrix.python-version }}" - name: "Install dependencies" run: "scripts/install" + - name: "Run linting checks" + run: "scripts/check" + - name: "Build package & docs" + run: "scripts/build" - name: "Run tests" run: "scripts/test" diff --git a/.gitignore b/.gitignore index 7b5d4318c..bff8fa258 100644 --- a/.gitignore +++ b/.gitignore @@ -3,5 +3,11 @@ test.db .coverage .pytest_cache/ .mypy_cache/ -starlette.egg-info/ -venv/ +__pycache__/ +htmlcov/ +site/ +*.egg-info/ +venv*/ +.python-version +build/ +dist/ diff --git a/requirements.txt b/requirements.txt index a044ee68f..4d4e205fa 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,6 +12,7 @@ ujson autoflake black==20.8b1 databases[sqlite] +flake8 isort==5.* mypy pytest @@ -22,3 +23,7 @@ pytest-asyncio mkdocs mkdocs-material mkautodoc + +# Packaging +twine +wheel diff --git a/scripts/README.md b/scripts/README.md index 84015423f..7388eac4b 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -2,7 +2,10 @@ * `scripts/install` - Install dependencies in a virtual environment. * `scripts/test` - Run the test suite. -* `scripts/lint` - Run the code linting. +* `scripts/lint` - Run the automated code linting/formatting tools. +* `scripts/check` - Run the code linting, checking that it passes. +* `scripts/coverage` - Check that code coverage is complete. +* `scripts/build` - Build source and wheel packages. * `scripts/publish` - Publish the latest version to PyPI. Styled after GitHub's ["Scripts to Rule Them All"](https://github.com/github/scripts-to-rule-them-all). diff --git a/scripts/build b/scripts/build new file mode 100755 index 000000000..1c47d2cc2 --- /dev/null +++ b/scripts/build @@ -0,0 +1,13 @@ +#!/bin/sh -e + +if [ -d 'venv' ] ; then + PREFIX="venv/bin/" +else + PREFIX="" +fi + +set -x + +${PREFIX}python setup.py sdist bdist_wheel +${PREFIX}twine check dist/* +${PREFIX}mkdocs build diff --git a/scripts/check b/scripts/check new file mode 100755 index 000000000..23d50c7c3 --- /dev/null +++ b/scripts/check @@ -0,0 +1,14 @@ +#!/bin/sh -e + +export PREFIX="" +if [ -d 'venv' ] ; then + export PREFIX="venv/bin/" +fi +export SOURCE_FILES="starlette tests" + +set -x + +${PREFIX}isort --check --diff --project=starlette $SOURCE_FILES +${PREFIX}black --check --diff $SOURCE_FILES +${PREFIX}flake8 $SOURCE_FILES +${PREFIX}mypy $SOURCE_FILES diff --git a/scripts/coverage b/scripts/coverage new file mode 100755 index 000000000..e871360d1 --- /dev/null +++ b/scripts/coverage @@ -0,0 +1,10 @@ +#!/bin/sh -e + +export PREFIX="" +if [ -d 'venv' ] ; then + export PREFIX="venv/bin/" +fi + +set -x + +${PREFIX}coverage report --show-missing --skip-covered --fail-under=100 diff --git a/scripts/docs b/scripts/docs new file mode 100755 index 000000000..4ac3beb7a --- /dev/null +++ b/scripts/docs @@ -0,0 +1,10 @@ +#!/bin/sh -e + +export PREFIX="" +if [ -d 'venv' ] ; then + export PREFIX="venv/bin/" +fi + +set -x + +${PREFIX}mkdocs serve diff --git a/scripts/lint b/scripts/lint index 26fbed90e..92e121691 100755 --- a/scripts/lint +++ b/scripts/lint @@ -4,11 +4,10 @@ export PREFIX="" if [ -d 'venv' ] ; then export PREFIX="venv/bin/" fi +export SOURCE_FILES="starlette tests" set -x -${PREFIX}mypy starlette --ignore-missing-imports --disallow-untyped-defs -${PREFIX}autoflake --in-place --recursive starlette tests setup.py -${PREFIX}black starlette tests setup.py -${PREFIX}isort --profile=black --combine-as starlette tests setup.py -${PREFIX}mypy starlette --ignore-missing-imports --disallow-untyped-defs +${PREFIX}autoflake --in-place --recursive $SOURCE_FILES +${PREFIX}isort --project=starlette $SOURCE_FILES +${PREFIX}black $SOURCE_FILES diff --git a/scripts/publish b/scripts/publish index 4c1b44805..667103d62 100755 --- a/scripts/publish +++ b/scripts/publish @@ -1,7 +1,6 @@ #!/bin/sh -e VERSION_FILE="starlette/__init__.py" -PYTHONPATH=. if [ -d 'venv' ] ; then PREFIX="venv/bin/" @@ -23,7 +22,5 @@ fi set -x -${PREFIX}pip install twine wheel mkdocs mkdocs-material mkautodoc -${PREFIX}python setup.py sdist bdist_wheel ${PREFIX}twine upload dist/* ${PREFIX}mkdocs gh-deploy --force diff --git a/scripts/test b/scripts/test index 366c238e3..f9c991723 100755 --- a/scripts/test +++ b/scripts/test @@ -1,17 +1,18 @@ -#!/bin/sh -e +#!/bin/sh export PREFIX="" if [ -d 'venv' ] ; then export PREFIX="venv/bin/" fi -export VERSION_SCRIPT="import sys; print('%s.%s' % sys.version_info[0:2])" -export PYTHON_VERSION=`python -c "$VERSION_SCRIPT"` +set -ex -set -x +if [ -z $GITHUB_ACTIONS ]; then + scripts/check +fi + +${PREFIX}pytest $@ -PYTHONPATH=. ${PREFIX}pytest --ignore venv --cov-config tests/.ignore_lifespan -W ignore::DeprecationWarning --cov=starlette --cov=tests --cov-fail-under=100 --cov-report=term-missing ${@} -${PREFIX}mypy starlette --ignore-missing-imports --disallow-untyped-defs -${PREFIX}autoflake --recursive starlette tests setup.py -${PREFIX}isort --profile=black --combine-as --check starlette tests setup.py -${PREFIX}black starlette tests setup.py --check +if [ -z $GITHUB_ACTIONS ]; then + scripts/coverage +fi diff --git a/setup.cfg b/setup.cfg new file mode 100644 index 000000000..fbbc3e200 --- /dev/null +++ b/setup.cfg @@ -0,0 +1,19 @@ +[flake8] +ignore = W503, E203, B305 +max-line-length = 88 + +[mypy] +disallow_untyped_defs = True +ignore_missing_imports = True + +[mypy-tests.*] +disallow_untyped_defs = False +# https://github.com/encode/starlette/issues/1045 +# check_untyped_defs = True + +[tool:isort] +profile = black +combine_as_imports = True + +[tool:pytest] +addopts = --cov-report= --cov=starlette --cov=tests -rxXs diff --git a/starlette/applications.py b/starlette/applications.py index 806b56261..0da20b691 100644 --- a/starlette/applications.py +++ b/starlette/applications.py @@ -25,8 +25,9 @@ class Starlette: with handled exception cases occuring in the routing or endpoints. * **exception_handlers** - A dictionary mapping either integer status codes, or exception class types onto callables which handle the exceptions. - Exception handler callables should be of the form `handler(request, exc) -> response` - and may be be either standard functions, or async functions. + Exception handler callables should be of the form + `handler(request, exc) -> response` and may be be either standard functions, or + async functions. * **on_startup** - A list of callables to run on application startup. Startup handler callables do not take any arguments, and may be be either standard functions, or async functions. diff --git a/starlette/config.py b/starlette/config.py index 114519a4c..782728aa3 100644 --- a/starlette/config.py +++ b/starlette/config.py @@ -24,14 +24,16 @@ def __getitem__(self, key: typing.Any) -> typing.Any: def __setitem__(self, key: typing.Any, value: typing.Any) -> None: if key in self._has_been_read: raise EnvironError( - f"Attempting to set environ['{key}'], but the value has already been read." + f"Attempting to set environ['{key}'], but the value has already been " + "read." ) self._environ.__setitem__(key, value) def __delitem__(self, key: typing.Any) -> None: if key in self._has_been_read: raise EnvironError( - f"Attempting to delete environ['{key}'], but the value has already been read." + f"Attempting to delete environ['{key}'], but the value has already " + "been read." ) self._environ.__delitem__(key) diff --git a/starlette/graphql.py b/starlette/graphql.py index b5eeeafa2..e67be3f1b 100644 --- a/starlette/graphql.py +++ b/starlette/graphql.py @@ -275,4 +275,4 @@ async def handle_graphiql(self, request: Request) -> Response: -""" +""" # noqa: E501 diff --git a/starlette/middleware/cors.py b/starlette/middleware/cors.py index 338aee863..0c9bdfb38 100644 --- a/starlette/middleware/cors.py +++ b/starlette/middleware/cors.py @@ -106,8 +106,8 @@ def preflight_response(self, request_headers: Headers) -> Response: if self.is_allowed_origin(origin=requested_origin): if not self.allow_all_origins: - # If self.allow_all_origins is True, then the "Access-Control-Allow-Origin" - # header is already set to "*". + # If self.allow_all_origins is True, then the + # "Access-Control-Allow-Origin" header is already set to "*". # If we only allow specific origins, then we have to mirror back # the Origin header in the response. headers["Access-Control-Allow-Origin"] = requested_origin diff --git a/starlette/middleware/errors.py b/starlette/middleware/errors.py index f1f8bd3d8..df1a38b2b 100644 --- a/starlette/middleware/errors.py +++ b/starlette/middleware/errors.py @@ -109,7 +109,7 @@

{code_context}
-""" +""" # noqa: E501 LINE = """

@@ -200,10 +200,12 @@ def generate_frame_html(self, frame: inspect.FrameInfo, is_collapsed: bool) -> s ) values = { - # HTML escape - filename could contain < or >, especially if it's a virtual file e.g. in the REPL + # HTML escape - filename could contain < or >, especially if it's a virtual + # file e.g. in the REPL "frame_filename": html.escape(frame.filename), "frame_lineno": frame.lineno, - # HTML escape - if you try very hard it's possible to name a function with < or > + # HTML escape - if you try very hard it's possible to name a function with < + # or > "frame_name": html.escape(frame.function), "code_context": code_context, "collapsed": "collapsed" if is_collapsed else "", @@ -226,7 +228,10 @@ def generate_html(self, exc: Exception, limit: int = 7) -> str: is_collapsed = True # escape error class and text - error = f"{html.escape(traceback_obj.exc_type.__name__)}: {html.escape(str(traceback_obj))}" + error = ( + f"{html.escape(traceback_obj.exc_type.__name__)}: " + f"{html.escape(str(traceback_obj))}" + ) return TEMPLATE.format(styles=STYLES, js=JS, error=error, exc_html=exc_html) diff --git a/starlette/middleware/wsgi.py b/starlette/middleware/wsgi.py index 8f7d92711..a552b4180 100644 --- a/starlette/middleware/wsgi.py +++ b/starlette/middleware/wsgi.py @@ -44,7 +44,8 @@ def build_environ(scope: Scope, body: bytes) -> dict: corrected_name = "CONTENT_TYPE" else: corrected_name = f"HTTP_{name}".upper().replace("-", "_") - # HTTPbis say only ASCII chars are allowed in headers, but we latin1 just in case + # HTTPbis say only ASCII chars are allowed in headers, but we latin1 just in + # case value = value.decode("latin1") if corrected_name in environ: value = environ[corrected_name] + "," + value diff --git a/starlette/requests.py b/starlette/requests.py index 56a0c5a9a..ab6f51424 100644 --- a/starlette/requests.py +++ b/starlette/requests.py @@ -153,7 +153,8 @@ def state(self) -> State: if not hasattr(self, "_state"): # Ensure 'state' has an empty dict if it's not already populated. self.scope.setdefault("state", {}) - # Create a state instance with a reference to the dict in which it should store info + # Create a state instance with a reference to the dict in which it should + # store info self._state = State(self.scope["state"]) return self._state diff --git a/starlette/routing.py b/starlette/routing.py index ac48169b9..75aa177a4 100644 --- a/starlette/routing.py +++ b/starlette/routing.py @@ -520,20 +520,20 @@ async def lifespan(self, scope: Scope, receive: Receive, send: Send) -> None: """ first = True app = scope.get("app") - message = await receive() + await receive() try: if inspect.isasyncgenfunction(self.lifespan_context): async for item in self.lifespan_context(app): assert first, "Lifespan context yielded multiple times." first = False await send({"type": "lifespan.startup.complete"}) - message = await receive() + await receive() else: for item in self.lifespan_context(app): # type: ignore assert first, "Lifespan context yielded multiple times." first = False await send({"type": "lifespan.startup.complete"}) - message = await receive() + await receive() except BaseException: if first: exc_text = traceback.format_exc() diff --git a/starlette/staticfiles.py b/starlette/staticfiles.py index 41df98062..8822efd10 100644 --- a/starlette/staticfiles.py +++ b/starlette/staticfiles.py @@ -147,7 +147,8 @@ async def lookup_path( full_path = os.path.realpath(os.path.join(directory, path)) directory = os.path.realpath(directory) if os.path.commonprefix([full_path, directory]) != directory: - # Don't allow misbehaving clients to break out of the static files directory. + # Don't allow misbehaving clients to break out of the static files + # directory. continue try: stat_result = await aio_stat(full_path) diff --git a/tests/middleware/test_cors.py b/tests/middleware/test_cors.py index a5b6e6235..2048cb11e 100644 --- a/tests/middleware/test_cors.py +++ b/tests/middleware/test_cors.py @@ -181,7 +181,7 @@ def test_cors_allow_origin_regex_fullmatch(): app.add_middleware( CORSMiddleware, allow_headers=["X-Example", "Content-Type"], - allow_origin_regex="https://.*\.example.org", + allow_origin_regex=r"https://.*\.example.org", ) @app.route("/") diff --git a/tests/test_authentication.py b/tests/test_authentication.py index 4c0f57ea9..3373f67c5 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -130,7 +130,7 @@ async def app(request): @app.route("/dashboard/decorated") @async_inject_decorator(additional="payload") @requires("authenticated") -async def decorated_sync(request, additional): +async def decorated_async(request, additional): return JSONResponse( { "authenticated": request.user.is_authenticated, @@ -176,7 +176,7 @@ def app(websocket): @app.websocket_route("/ws/decorated") @ws_inject_decorator(additional="payload") @requires("authenticated") -async def websocket_endpoint(websocket, additional): +async def websocket_endpoint_decorated(websocket, additional): await websocket.accept() await websocket.send_json( { diff --git a/tests/test_formparsers.py b/tests/test_formparsers.py index 17996de2c..73a720fd1 100644 --- a/tests/test_formparsers.py +++ b/tests/test_formparsers.py @@ -192,7 +192,9 @@ def test_multipart_request_mixed_files_and_data(tmpdir): b"--a7f7ac8d4e2e437c877bb7b8d7cc549c--\r\n" ), headers={ - "Content-Type": "multipart/form-data; boundary=a7f7ac8d4e2e437c877bb7b8d7cc549c" + "Content-Type": ( + "multipart/form-data; boundary=a7f7ac8d4e2e437c877bb7b8d7cc549c" + ) }, ) assert response.json() == { @@ -213,13 +215,16 @@ def test_multipart_request_with_charset_for_filename(tmpdir): data=( # file b"--a7f7ac8d4e2e437c877bb7b8d7cc549c\r\n" - b'Content-Disposition: form-data; name="file"; filename="\xe6\x96\x87\xe6\x9b\xb8.txt"\r\n' + b'Content-Disposition: form-data; name="file"; filename="\xe6\x96\x87\xe6\x9b\xb8.txt"\r\n' # noqa: E501 b"Content-Type: text/plain\r\n\r\n" b"\r\n" b"--a7f7ac8d4e2e437c877bb7b8d7cc549c--\r\n" ), headers={ - "Content-Type": "multipart/form-data; charset=utf-8; boundary=a7f7ac8d4e2e437c877bb7b8d7cc549c" + "Content-Type": ( + "multipart/form-data; charset=utf-8; " + "boundary=a7f7ac8d4e2e437c877bb7b8d7cc549c" + ) }, ) assert response.json() == { @@ -238,13 +243,15 @@ def test_multipart_request_without_charset_for_filename(tmpdir): data=( # file b"--a7f7ac8d4e2e437c877bb7b8d7cc549c\r\n" - b'Content-Disposition: form-data; name="file"; filename="\xe7\x94\xbb\xe5\x83\x8f.jpg"\r\n' + b'Content-Disposition: form-data; name="file"; filename="\xe7\x94\xbb\xe5\x83\x8f.jpg"\r\n' # noqa: E501 b"Content-Type: image/jpeg\r\n\r\n" b"\r\n" b"--a7f7ac8d4e2e437c877bb7b8d7cc549c--\r\n" ), headers={ - "Content-Type": "multipart/form-data; boundary=a7f7ac8d4e2e437c877bb7b8d7cc549c" + "Content-Type": ( + "multipart/form-data; boundary=a7f7ac8d4e2e437c877bb7b8d7cc549c" + ) }, ) assert response.json() == { @@ -268,7 +275,10 @@ def test_multipart_request_with_encoded_value(tmpdir): b"--20b303e711c4ab8c443184ac833ab00f--\r\n" ), headers={ - "Content-Type": "multipart/form-data; charset=utf-8; boundary=20b303e711c4ab8c443184ac833ab00f" + "Content-Type": ( + "multipart/form-data; charset=utf-8; " + "boundary=20b303e711c4ab8c443184ac833ab00f" + ) }, ) assert response.json() == {"value": "Transférer"} diff --git a/tests/test_requests.py b/tests/test_requests.py index c5e50edbe..a83a2c480 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -287,9 +287,9 @@ async def app(scope, receive, send): def test_cookie_lenient_parsing(): """ - The following test is based on a cookie set by Okta, a well-known authorization service. - It turns out that it's common practice to set cookies that would be invalid according to - the spec. + The following test is based on a cookie set by Okta, a well-known authorization + service. It turns out that it's common practice to set cookies that would be + invalid according to the spec. """ tough_cookie = ( "provider-oauth-nonce=validAsciiblabla; " diff --git a/tests/test_routing.py b/tests/test_routing.py index 51c32b0e3..bbccd85b6 100644 --- a/tests/test_routing.py +++ b/tests/test_routing.py @@ -442,8 +442,12 @@ def test_url_for_with_root_path(): } +async def stub_app(scope, receive, send): + pass # pragma: no cover + + double_mount_routes = [ - Mount("/mount", name="mount", routes=[Mount("/static", ..., name="static")]), + Mount("/mount", name="mount", routes=[Mount("/static", stub_app, name="static")]), ]