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

Add precommit and run precommit on all files #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
/build/

/dist/
/venv/
30 changes: 30 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
- repo: https://github.com/pre-commit/pre-commit-hooks.git
sha: v0.7.1
hooks:
- id: autopep8-wrapper
- id: check-added-large-files
- id: check-ast
- id: check-byte-order-marker
- id: check-case-conflict
- id: check-docstring-first
- id: check-json
- id: check-merge-conflict
- id: check-yaml
- id: debug-statements
- id: double-quote-string-fixer
- id: end-of-file-fixer
- id: flake8
- id: fix-encoding-pragma
- id: name-tests-test
- id: pretty-format-json
args: ['--autofix']
- id: requirements-txt-fixer
- id: trailing-whitespace
- repo: https://github.com/asottile/pyupgrade
sha: v1.0.4
hooks:
- id: pyupgrade
- repo: https://github.com/asottile/reorder_python_imports
sha: v0.3.2
hooks:
- id: reorder-python-imports
Copy link
Member

@syrusakbary syrusakbary Mar 18, 2017

Choose a reason for hiding this comment

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

I've been using isort for checking the import ordering: https://github.com/graphql-python/flask-graphql/blob/master/tox.ini#L30.
Also being used in the automatic linters in graphql-core https://github.com/graphql-python/graphql-core/blob/master/bin/autolinter#L7, and graphene https://github.com/graphql-python/graphene/blob/master/bin/autolinter#L7.

I kind of prefer it over render-python-imports as it seems it does a smarter import grouping, not requiring a import statement per imported var.

If we could use isort instead of render-python-imports would be great! :)

1 change: 1 addition & 0 deletions flask_graphql/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-
from .blueprint import GraphQL
from .graphqlview import GraphQLView

Expand Down
1 change: 1 addition & 0 deletions flask_graphql/blueprint.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-
import warnings

from flask import Blueprint
Expand Down
13 changes: 9 additions & 4 deletions flask_graphql/graphqlview.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,21 @@
# -*- coding: utf-8 -*-
import json

import six
from flask import Response, request
from flask import request
from flask import Response
from flask.views import View
from werkzeug.exceptions import BadRequest, MethodNotAllowed

from graphql import Source, execute, parse, validate
from graphql import execute
from graphql import parse
from graphql import Source
from graphql import validate
from graphql.error import format_error as format_graphql_error
from graphql.error import GraphQLError
from graphql.execution import ExecutionResult
from graphql.type.schema import GraphQLSchema
from graphql.utils.get_operation_ast import get_operation_ast
from werkzeug.exceptions import BadRequest
from werkzeug.exceptions import MethodNotAllowed

from .render_graphiql import render_graphiql

Expand Down
1 change: 1 addition & 0 deletions flask_graphql/render_graphiql.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-
from flask import render_template_string


Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pre-commit==0.13.3

Choose a reason for hiding this comment

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

I'd probably call this file requirements-dev.txt since you don't actually need pre-commit to be installed in production (and that's what requirements.txt implies, even if you're not actually doing it in this case).

Choose a reason for hiding this comment

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

Also, we usually don't pin specific versions for our dev dependencies, but that's totally up to you / the project's stance on that.

Copy link
Member

@syrusakbary syrusakbary Mar 18, 2017

Choose a reason for hiding this comment

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

Maybe adding something like the following in the README will actually add more visibility into Yelp's pre-commit project and provide a better understanding to the developer, rather than using a requirements.txt file that might confuse collaborators :)


Collaborate

This project is using pre-commit to help the developer run pre-commit hooks easily, allowing to autoformat Python files, reorder the imports, and other various checks.

You can easily start using it via pip install pre-commit.


Thoughts?

4 changes: 3 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
from setuptools import setup, find_packages
# -*- coding: utf-8 -*-
from setuptools import find_packages
from setuptools import setup

required_packages = ['graphql-core>=1.0', 'flask>=0.7.0']

Expand Down
4 changes: 3 additions & 1 deletion tests/app.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# -*- coding: utf-8 -*-
from flask import Flask
from flask_graphql import GraphQLView

from .schema import Schema
from flask_graphql import GraphQLView


def create_app(path='/graphql', **kwargs):
Expand Down
8 changes: 6 additions & 2 deletions tests/schema.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
from graphql.type.definition import GraphQLArgument, GraphQLField, GraphQLNonNull, GraphQLObjectType
# -*- coding: utf-8 -*-
from graphql.type.definition import GraphQLArgument
from graphql.type.definition import GraphQLField
from graphql.type.definition import GraphQLNonNull
from graphql.type.definition import GraphQLObjectType
from graphql.type.scalars import GraphQLString
from graphql.type.schema import GraphQLSchema


def resolve_raises(*_):
raise Exception("Throws!")
raise Exception('Throws!')


QueryRootType = GraphQLObjectType(
Expand Down
5 changes: 3 additions & 2 deletions tests/test_graphiqlview.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# -*- coding: utf-8 -*-
import pytest
from flask import url_for

from .app import create_app
from flask import url_for


@pytest.fixture
Expand All @@ -23,6 +24,6 @@ def test_graphiql_renders_pretty(client):
' "test": "Hello World"\n'
' }\n'
'}'
).replace("\"","\\\"").replace("\n","\\n")
).replace("\"", "\\\"").replace('\n', '\\n')

assert pretty_response in response.data.decode('utf-8')
70 changes: 40 additions & 30 deletions tests/test_graphqlview.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import pytest
# -*- coding: utf-8 -*-
import json

import pytest

try:
from StringIO import StringIO
except ImportError:
Expand All @@ -19,6 +21,7 @@
def app():
return create_app()


def url_string(**url_params):
string = url_for('graphql')

Expand All @@ -41,19 +44,19 @@ def test_allows_get_with_query_param(client):

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello World"}
'data': {'test': 'Hello World'}
}


def test_allows_get_with_variable_values(client):
response = client.get(url_string(
query='query helloWho($who: String){ test(who: $who) }',
variables=json.dumps({'who': "Dolly"})
variables=json.dumps({'who': 'Dolly'})
))

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello Dolly"}
'data': {'test': 'Hello Dolly'}
}


Expand Down Expand Up @@ -163,7 +166,7 @@ def test_allows_mutation_to_exist_within_a_get(client):

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello World"}
'data': {'test': 'Hello World'}
}


Expand All @@ -172,12 +175,16 @@ def test_allows_post_with_json_encoding(client):

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello World"}
'data': {'test': 'Hello World'}
}


def test_allows_sending_a_mutation_via_post(client):
response = client.post(url_string(), data=j(query='mutation TestMutation { writeTest { test } }'), content_type='application/json')
response = client.post(
url_string(),
data=j(query='mutation TestMutation { writeTest { test } }'),
content_type='application/json'
)

assert response.status_code == 200
assert response_json(response) == {
Expand All @@ -186,87 +193,91 @@ def test_allows_sending_a_mutation_via_post(client):


def test_allows_post_with_url_encoding(client):
response = client.post(url_string(), data=urlencode(dict(query='{test}')), content_type='application/x-www-form-urlencoded')
response = client.post(
url_string(),
data=urlencode(dict(query='{test}')),
content_type='application/x-www-form-urlencoded'
)

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello World"}
'data': {'test': 'Hello World'}
}


def test_supports_post_json_query_with_string_variables(client):
response = client.post(url_string(), data=j(
query='query helloWho($who: String){ test(who: $who) }',
variables=json.dumps({'who': "Dolly"})
variables=json.dumps({'who': 'Dolly'})
), content_type='application/json')

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello Dolly"}
'data': {'test': 'Hello Dolly'}
}


def test_supports_post_json_query_with_json_variables(client):
response = client.post(url_string(), data=j(
query='query helloWho($who: String){ test(who: $who) }',
variables={'who': "Dolly"}
variables={'who': 'Dolly'}
), content_type='application/json')

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello Dolly"}
'data': {'test': 'Hello Dolly'}
}


def test_supports_post_url_encoded_query_with_string_variables(client):
response = client.post(url_string(), data=urlencode(dict(
query='query helloWho($who: String){ test(who: $who) }',
variables=json.dumps({'who': "Dolly"})
variables=json.dumps({'who': 'Dolly'})
)), content_type='application/x-www-form-urlencoded')

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello Dolly"}
'data': {'test': 'Hello Dolly'}
}


def test_supports_post_json_quey_with_get_variable_values(client):
response = client.post(url_string(
variables=json.dumps({'who': "Dolly"})
variables=json.dumps({'who': 'Dolly'})
), data=j(
query='query helloWho($who: String){ test(who: $who) }',
), content_type='application/json')

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello Dolly"}
'data': {'test': 'Hello Dolly'}
}


def test_post_url_encoded_query_with_get_variable_values(client):
response = client.post(url_string(
variables=json.dumps({'who': "Dolly"})
variables=json.dumps({'who': 'Dolly'})
), data=urlencode(dict(
query='query helloWho($who: String){ test(who: $who) }',
)), content_type='application/x-www-form-urlencoded')

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello Dolly"}
'data': {'test': 'Hello Dolly'}
}


def test_supports_post_raw_text_query_with_get_variable_values(client):
response = client.post(url_string(
variables=json.dumps({'who': "Dolly"})
variables=json.dumps({'who': 'Dolly'})
),
data='query helloWho($who: String){ test(who: $who) }',
content_type='application/graphql'
)

assert response.status_code == 200
assert response_json(response) == {
'data': {'test': "Hello Dolly"}
'data': {'test': 'Hello Dolly'}
}


Expand Down Expand Up @@ -396,7 +407,7 @@ def test_handles_incomplete_json_bodies(client):

def test_handles_plain_post_text(client):
response = client.post(url_string(
variables=json.dumps({'who': "Dolly"})
variables=json.dumps({'who': 'Dolly'})
),
data='query helloWho($who: String){ test(who: $who) }',
content_type='text/plain'
Expand Down Expand Up @@ -438,11 +449,10 @@ def test_passes_request_into_request_context(client):
}


@pytest.mark.parametrize('app', [create_app(context="CUSTOM CONTEXT")])
@pytest.mark.parametrize('app', [create_app(context='CUSTOM CONTEXT')])
def test_supports_pretty_printing(client):
response = client.get(url_string(query='{context}'))


assert response.status_code == 200
assert response_json(response) == {
'data': {
Expand All @@ -455,7 +465,7 @@ def test_post_multipart_data(client):
query = 'mutation TestMutation { writeTest { test } }'
response = client.post(
url_string(),
data= {
data={
'query': query,
'file': (StringIO(), 'text1.txt'),
},
Expand All @@ -477,7 +487,7 @@ def test_batch_allows_post_with_json_encoding(client):
assert response.status_code == 200
assert response_json(response) == [{
'id': 1,
'payload': { 'data': {'test': "Hello World"} },
'payload': {'data': {'test': 'Hello World'}},
'status': 200,
}]

Expand All @@ -489,19 +499,19 @@ def test_batch_supports_post_json_query_with_json_variables(client):
data=jl(
id=1,
query='query helloWho($who: String){ test(who: $who) }',
variables={'who': "Dolly"}
variables={'who': 'Dolly'}
),
content_type='application/json'
)

assert response.status_code == 200
assert response_json(response) == [{
'id': 1,
'payload': { 'data': {'test': "Hello Dolly"} },
'payload': {'data': {'test': 'Hello Dolly'}},
'status': 200,
}]


@pytest.mark.parametrize('app', [create_app(batch=True)])
def test_batch_allows_post_with_operation_name(client):
response = client.post(
Expand Down
3 changes: 3 additions & 0 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ deps =
Flask>=0.10.0
commands =
isort --check-only flask_graphql/ -rc

[pep8]
max-line-length=120