#19 clean up the code to make tox happy
Merged 7 years ago by mjia. Opened 7 years ago by mjia.
mjia/waiverdb fix_tox  into  master

file modified
+1 -5
@@ -9,8 +9,4 @@ 

        author_email='qa-devel@lists.fedoraproject.org',

        license='GPLv2+',

        packages=['waiverdb'],

-       package_dir={'waiverdb': 'waiverdb'},

-       #entry_points={

-       #    #TODO: register messaging plugins

-       #},

- )

+       package_dir={'waiverdb': 'waiverdb'})

file modified
+4 -1
@@ -10,11 +10,11 @@ 

  # GNU General Public License for more details.

  #

  

- import os

  import pytest

  

  from waiverdb.app import create_app, init_db

  

+ 

  @pytest.fixture(scope='session')

  def app(request):

      app = create_app('waiverdb.config.TestingConfig')
@@ -28,12 +28,14 @@ 

      request.addfinalizer(teardown)

      return app

  

+ 

  @pytest.fixture(scope='session')

  def db(app):

      """Session-wide test database."""

      db = init_db(app)

      return db

  

+ 

  @pytest.yield_fixture

  def session(db, monkeypatch):

      """Patch Flask-SQLAlchemy to use a specific connection"""
@@ -49,6 +51,7 @@ 

      transaction.rollback()

      connection.close()

  

+ 

  @pytest.yield_fixture

  def client(app):

      """A Flask test client. An instance of :class:`flask.testing.TestClient`

file modified
+50 -38
@@ -9,12 +9,12 @@ 

  # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

  # GNU General Public License for more details.

  

- import pytest

  import json

  from .utils import create_waiver

  import datetime

  from mock import patch

  

+ 

  @patch('waiverdb.auth.get_user', return_value=('foo', {}))

  def test_create_waiver(mocked_get_user, client, session, monkeypatch):

      data = {
@@ -24,145 +24,157 @@ 

          'comment': 'it broke',

      }

      r = client.post('/api/v1.0/waivers/', data=json.dumps(data),

-             content_type='application/json')

-     res_data = json.loads(r.data)

+                     content_type='application/json')

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 201

      assert res_data['username'] == 'foo'

      assert res_data['result_id'] == 123

      assert res_data['product_version'] == 'fool-1'

-     assert res_data['waived'] == True

+     assert res_data['waived'] is True

      assert res_data['comment'] == 'it broke'

  

+ 

  @patch('waiverdb.auth.get_user', return_value=('foo', {}))

  def test_create_waiver_with_malformed_data(mocked_get_user, client):

      data = {

          'result_id': 'wrong id',

      }

      r = client.post('/api/v1.0/waivers/', data=json.dumps(data),

-             content_type='application/json')

-     res_data = json.loads(r.data)

+                     content_type='application/json')

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 400

      assert 'invalid literal for int()' in res_data['message']['result_id']

  

+ 

  def test_get_waiver(client, session):

      # create a new waiver

      waiver = create_waiver(session, result_id=123, username='foo',

-             product_version='foo-1', comment='bla bla bla')

+                            product_version='foo-1', comment='bla bla bla')

      r = client.get('/api/v1.0/waivers/%s' % waiver.id)

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert res_data['username'] == waiver.username

      assert res_data['result_id'] == waiver.result_id

      assert res_data['product_version'] == waiver.product_version

-     assert res_data['waived'] == True

+     assert res_data['waived'] is True

      assert res_data['comment'] == waiver.comment

  

+ 

  def test_404_for_nonexistent_waiver(client, session):

      r = client.get('/api/v1.0/waivers/foo')

      assert r.status_code == 404

  

+ 

  def test_get_waivers(client, session):

-     for i in range(0,10):

+     for i in range(0, 10):

          create_waiver(session, result_id=i, username='foo %d' % i,

-                 product_version='foo-%d' % i, comment='bla bla bla')

+                       product_version='foo-%d' % i, comment='bla bla bla')

      r = client.get('/api/v1.0/waivers/')

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert len(res_data['data']) == 10

  

+ 

  def test_pagination_waivers(client, session):

-     for i in range(0,10):

+     for i in range(0, 30):

          create_waiver(session, result_id=i, username='foo %d' % i,

-                 product_version='foo-%d' % i, comment='bla bla bla')

-     r = client.get('/api/v1.0/waivers/?page=2&limit=1')

-     res_data = json.loads(r.data)

+                       product_version='foo-%d' % i, comment='bla bla bla')

+     r = client.get('/api/v1.0/waivers/?page=2')

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

-     assert len(res_data['data']) == 1

-     assert '/waivers/?limit=1&page=1' in res_data['prev']

-     assert '/waivers/?limit=1&page=3' in res_data['next']

-     assert '/waivers/?limit=1&page=1' in res_data['first']

-     assert '/waivers/?limit=1&page=10' in res_data['last']

+     assert len(res_data['data']) == 10

+     assert '/waivers/?page=1' in res_data['prev']

+     assert '/waivers/?page=3' in res_data['next']

+     assert '/waivers/?page=1' in res_data['first']

+     assert '/waivers/?page=3' in res_data['last']

+ 

  

  def test_obsolete_waivers_are_excluded_by_default(client, session):

-     old_waiver = create_waiver(session, result_id=123, username='foo',

-             product_version='foo-1')

+     create_waiver(session, result_id=123, username='foo',

+                   product_version='foo-1')

      new_waiver = create_waiver(session, result_id=123, username='foo',

-             product_version='foo-1', waived=False)

+                                product_version='foo-1', waived=False)

      r = client.get('/api/v1.0/waivers/')

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert len(res_data['data']) == 1

      assert res_data['data'][0]['id'] == new_waiver.id

      assert res_data['data'][0]['waived'] == new_waiver.waived

  

+ 

  def test_get_obsolete_waivers(client, session):

      old_waiver = create_waiver(session, result_id=123, username='foo',

-             product_version='foo-1')

+                                product_version='foo-1')

      new_waiver = create_waiver(session, result_id=123, username='foo',

-             product_version='foo-1', waived=False)

+                                product_version='foo-1', waived=False)

      r = client.get('/api/v1.0/waivers/?include_obsolete=1')

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert len(res_data['data']) == 2

-     assert res_data['data'][0]['id'] ==  new_waiver.id

-     assert res_data['data'][1]['id'] ==  old_waiver.id

+     assert res_data['data'][0]['id'] == new_waiver.id

+     assert res_data['data'][1]['id'] == old_waiver.id

+ 

  

  def test_filtering_waivers_by_result_id(client, session):

      create_waiver(session, result_id=123, username='foo-1', product_version='foo-1')

      create_waiver(session, result_id=234, username='foo-2', product_version='foo-1')

      r = client.get('/api/v1.0/waivers/?result_id=123')

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert len(res_data['data']) == 1

      assert res_data['data'][0]['result_id'] == 123

  

+ 

  def test_filtering_waivers_by_product_version(client, session):

      create_waiver(session, result_id=123, username='foo-1', product_version='release-1')

      create_waiver(session, result_id=124, username='foo-1', product_version='release-2')

      r = client.get('/api/v1.0/waivers/?product_version=release-1')

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert len(res_data['data']) == 1

      assert res_data['data'][0]['product_version'] == 'release-1'

  

+ 

  def test_filtering_waivers_by_username(client, session):

      create_waiver(session, result_id=123, username='foo', product_version='foo-1')

      create_waiver(session, result_id=124, username='bar', product_version='foo-2')

      r = client.get('/api/v1.0/waivers/?username=foo')

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert len(res_data['data']) == 1

      assert res_data['data'][0]['username'] == 'foo'

  

+ 

  def test_filtering_waivers_by_since(client, session):

      before1 = (datetime.datetime.utcnow() - datetime.timedelta(seconds=100)).isoformat()

      before2 = (datetime.datetime.utcnow() - datetime.timedelta(seconds=99)).isoformat()

      after = (datetime.datetime.utcnow() + datetime.timedelta(seconds=100)).isoformat()

      create_waiver(session, result_id=123, username='foo', product_version='foo-1')

      r = client.get('/api/v1.0/waivers/?since=%s' % before1)

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert len(res_data['data']) == 1

      assert res_data['data'][0]['result_id'] == 123

  

      r = client.get('/api/v1.0/waivers/?since=%s,%s' % (before1, after))

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert len(res_data['data']) == 1

      assert res_data['data'][0]['result_id'] == 123

  

      r = client.get('/api/v1.0/waivers/?since=%s' % (after))

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert len(res_data['data']) == 0

  

      r = client.get('/api/v1.0/waivers/?since=%s,%s' % (before1, before2))

-     res_data = json.loads(r.data)

+     res_data = json.loads(r.get_data(as_text=True))

      assert r.status_code == 200

      assert len(res_data['data']) == 0

  

+ 

  def test_jsonp(client, session):

      waiver = create_waiver(session, result_id=123, username='foo', product_version='foo-1')

      r = client.get('/api/v1.0/waivers/%s?callback=jsonpcallback' % waiver.id)

      assert r.mimetype == 'application/javascript'

-     assert 'jsonpcallback' in r.data

+     assert 'jsonpcallback' in r.get_data(as_text=True)

file modified
+5 -4
@@ -16,6 +16,7 @@ 

  from werkzeug.exceptions import Unauthorized

  import waiverdb.auth

  

+ 

  class TestKerberosAuthentication(object):

  

      def test_keytab_file_is_not_set_should_raise_error(self):
@@ -36,7 +37,7 @@ 

      @mock.patch('kerberos.authGSSServerClean')

      @mock.patch('kerberos.getServerPrincipalDetails')

      def test_authorized(self, principal, clean, name, response, step, init,

-         client, monkeypatch, session):

+                         client, monkeypatch, session):

          monkeypatch.setenv('KRB5_KTNAME', '/etc/foo.keytab')

          data = {

              'result_id': 123,
@@ -45,9 +46,9 @@ 

              'comment': 'it broke',

          }

          r = client.post('/api/v1.0/waivers/',  data=json.dumps(data),

-                 content_type='application/json',

-                 headers={'Authorization': 'Negotiate CTOKEN'})

+                         content_type='application/json',

+                         headers={'Authorization': 'Negotiate CTOKEN'})

          assert r.status_code == 201

          assert r.headers.get('WWW-Authenticate') == 'negotiate STOKEN'

-         res_data = json.loads(r.data)

+         res_data = json.loads(r.data.decode('utf-8'))

          assert res_data['username'] == 'foo'

file modified
+2 -1
@@ -12,8 +12,9 @@ 

  

  from waiverdb.models import Waiver

  

+ 

  def create_waiver(session, result_id, username, product_version, waived=True,

-     comment=None):

+                   comment=None):

      waiver = Waiver(result_id, username, product_version, waived, comment)

      session.add(waiver)

      session.flush()

file modified
+4 -2
@@ -12,7 +12,7 @@ 

  

  from flask import Blueprint, request

  from flask_restful import Resource, Api, reqparse, marshal_with

- from werkzeug.exceptions import HTTPException, BadRequest, NotFound

+ from werkzeug.exceptions import BadRequest, NotFound

  from sqlalchemy.sql.expression import func

  

  from waiverdb.models import db, Waiver
@@ -44,6 +44,7 @@ 

  RP['get_waivers'].add_argument('page', default=1, type=int, location='args')

  RP['get_waivers'].add_argument('limit', default=10, type=int, location='args')

  

+ 

  class WaiversResource(Resource):

      @jsonp

      def get(self):
@@ -75,7 +76,7 @@ 

          user, headers = waiverdb.auth.get_user(request)

          args = RP['create_waiver'].parse_args()

          waiver = Waiver(args['result_id'], user, args['product_version'], args['waived'],

-                 args['comment'])

+                         args['comment'])

          db.session.add(waiver)

          db.session.commit()

          return waiver, 201, headers
@@ -90,6 +91,7 @@ 

          except:

              raise NotFound('Waiver not found')

  

+ 

  # set up the Api resource routing here

  api.add_resource(WaiversResource, '/waivers/')

  api.add_resource(WaiverResource, '/waivers/<int:waiver_id>')

file modified
+3 -1
@@ -11,11 +11,11 @@ 

  

  import os

  from flask import Flask

- from flask_sqlalchemy import SQLAlchemy

  from waiverdb.logger import init_logging

  from waiverdb.api_v1 import api_v1

  from waiverdb.models import db

  

+ 

  def load_default_config(app):

      # Load default config, then override that with a config file

      if os.getenv('DEV') == 'true':
@@ -32,6 +32,7 @@ 

      if os.path.exists(config_file):

          app.config.from_pyfile(config_file)

  

+ 

  # applicaiton factory http://flask.pocoo.org/docs/0.12/patterns/appfactories/

  def create_app(config_obj=None):

      app = Flask(__name__)
@@ -51,6 +52,7 @@ 

      app.register_blueprint(api_v1, url_prefix="/api/v1.0")

      return app

  

+ 

  def init_db(app):

      with app.app_context():

          db.create_all()

file modified
+3 -1
@@ -21,7 +21,8 @@ 

  from socket import gethostname

  from werkzeug.exceptions import Unauthorized, Forbidden

  

- #Inspired by https://github.com/mkomitee/flask-kerberos/blob/master/flask_kerberos.py

+ 

+ # Inspired by https://github.com/mkomitee/flask-kerberos/blob/master/flask_kerberos.py

  class KerberosAuthenticate(object):

  

      def __init__(self):
@@ -88,6 +89,7 @@ 

              raise Forbidden("Invalid Kerberos ticket")

          return kerberos_user, kerberos_token

  

+ 

  def get_user(request):

      user = None

      headers = dict()

file modified
+3
@@ -12,6 +12,7 @@ 

  import logging

  import sys

  

+ 

  def log_to_stdout(app, level=logging.INFO):

      fmt = '[%(filename)s:%(lineno)d] ' if app.debug else '%(module)-12s '

      fmt += '%(asctime)s %(levelname)-7s %(message)s'
@@ -21,6 +22,7 @@ 

      stream_handler.setFormatter(logging.Formatter(fmt=fmt, datefmt=datefmt))

      app.logger.addHandler(stream_handler)

  

+ 

  def log_to_journal(app, level=logging.INFO):

      try:

          import systemd.journal
@@ -30,6 +32,7 @@ 

      journal_handler.setLevel(level)

      app.logger.addHandler(journal_handler)

  

+ 

  def init_logging(app):

      log_level = logging.DEBUG if app.debug else logging.INFO

      if app.config['JOURNAL_LOGGING']:

file modified
+2 -2
@@ -9,5 +9,5 @@ 

  # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the

  # GNU General Public License for more details.

  

- from .base import db

- from .waivers import Waiver

+ from .base import db  # noqa: F401

+ from .waivers import Waiver  # noqa: F401

@@ -12,6 +12,7 @@ 

  import datetime

  from .base import db

  

+ 

  class Waiver(db.Model):

      id = db.Column(db.Integer, primary_key=True)

      result_id = db.Column(db.Integer, nullable=False)

file modified
+7 -4
@@ -16,6 +16,7 @@ 

  from waiverdb.fields import waiver_fields

  from werkzeug.exceptions import NotFound

  

+ 

  def reqparse_since(since):

      """

      This parses the since(i.e. 2017-02-13T23:37:58.193281, 2017-02-16T23:37:58.193281)
@@ -33,15 +34,16 @@ 

          end = datetime.datetime.strptime(end, "%Y-%m-%dT%H:%M:%S.%f")

      return start, end

  

+ 

  def json_collection(query, page=1, limit=10):

      """

-     Helper function for Flask request handlers which want to return 

+     Helper function for Flask request handlers which want to return

      a collection of resources as JSON.

      """

      try:

          p = query.paginate(page, limit)

      except NotFound:

-         return {'data': [], 'prev': None, 'next': None, 'first': None, 'last':None}

+         return {'data': [], 'prev': None, 'next': None, 'first': None, 'last': None}

      pages = {'data': marshal(p.items, waiver_fields)}

      query_pairs = request.args.copy()

      if query_pairs:
@@ -49,18 +51,19 @@ 

          query_pairs.pop('page', default=None)

      if p.has_prev:

          pages['prev'] = url_for(request.endpoint, page=p.prev_num, _external=True,

-                 **query_pairs)

+                                 **query_pairs)

      else:

          pages['prev'] = None

      if p.has_next:

          pages['next'] = url_for(request.endpoint, page=p.next_num, _external=True,

-                 **query_pairs)

+                                 **query_pairs)

      else:

          pages['next'] = None

      pages['first'] = url_for(request.endpoint, page=1, _external=True, **query_pairs)

      pages['last'] = url_for(request.endpoint, page=p.pages, _external=True, **query_pairs)

      return pages

  

+ 

  def jsonp(func):

      """Wraps Jsonified output for JSONP requests."""

      @functools.wraps(func)

no initial comment

Sort of nitpicky, but I think you can have Flask automatically decode the data using the correct encoding scheme (as set in the HTTP header) with r.get_data(as_text=True). It is just a test and I doubt it'll get run on a system that's not using UTF-8 by default, but I think technically it's not safe to assume the data is UTF-8 encoded.

If you want to continue importing these to provide a nice package interface (I do like having the __init__.py define the public interface) you can mark the specific lines to be ignored by the linter:

from .base import db  # noqa: F401
from .waivers import Waiver  # noqa: F401

will explicitly ignore imports that are unused, or you can tell flake8 to ignore any error with a plain # noqa comment.

http://flake8.pycqa.org/en/latest/user/ignoring-errors.html#in-line-ignoring-errors for reference

Generally +1, but I recommend using the get_data API.

rebased

7 years ago

Yes! Looks good to me too. :+1:

Pull-Request has been merged by mjia

7 years ago