ci: Replace pep8 by flake8 and separate linting stage on Travis (#162)

Author: oakbani

.travis.yml

@@ -8,26 +8,31 @@ python:
   - "pypy"
   - "pypy3"
 install: "pip install -r requirements/core.txt;pip install -r requirements/test.txt"
-before_script: "pep8"
 addons:
   srcclr: true
 script: "nosetests --with-coverage --cover-package=optimizely"
 after_success:
   - coveralls
 
-# Integration tests need to run first to reset the PR build status to pending
+# Linting and Integration tests need to run first to reset the PR build status to pending.
 stages:
+  - 'Linting'
   - 'Integration tests'
   - 'Test'
 
 jobs:
   include:
-    - stage: 'Integration tests'
+    - stage: 'Linting'
+      language: python
+      python: "2.7"
+      install: "pip install flake8"
+      script: "flake8"
+      after_success: travis_terminate 0
+    - stage: 'Integration Tests'
       merge_mode: replace
       env: SDK=python
       cache: false
       language: python
-      before_install: skip
       install:
         - "pip install awscli"
       before_script:

CONTRIBUTING.rst

@@ -37,7 +37,7 @@ Pull request acceptance criteria
 Style
 -----
 
-We enforce PEP-8 rules with a few minor `deviations`_.
+We enforce Flake8 rules with a few minor `deviations`_.
 
 License
 -------

optimizely/helpers/condition.py

@@ -1,4 +1,4 @@
-# Copyright 2016, 2018, Optimizely
+# Copyright 2016, 2018-2019, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -12,7 +12,6 @@
 # limitations under the License.
 
 import json
-import numbers
 
 from six import string_types
 

optimizely/helpers/condition_tree_evaluator.py

@@ -1,4 +1,4 @@
-# Copyright 2018, Optimizely
+# Copyright 2018-2019, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -86,6 +86,7 @@ def not_evaluator(conditions, leaf_evaluator):
   result = evaluate(conditions[0], leaf_evaluator)
   return None if result is None else not result
 
+
 EVALUATORS_BY_OPERATOR_TYPE = {
   ConditionOperatorTypes.AND: and_evaluator,
   ConditionOperatorTypes.OR: or_evaluator,

requirements/test.txt

@@ -1,7 +1,7 @@
 coverage==4.0.3
+flake8==3.6.0
 funcsigs==0.4
 mock==1.3.0
 nose==1.3.7
-pep8==1.7.0
 python-coveralls==2.7.0
 tabulate==0.7.5

tests/benchmarking/benchmarking_tests.py

@@ -1,4 +1,4 @@
-# Copyright 2016, Optimizely
+# Copyright 2016, 2019, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -231,5 +231,6 @@ def run_benchmarking_tests():
 
   display_results(all_test_results_average, all_test_results_median)
 
+
 if __name__ == '__main__':
   run_benchmarking_tests()

tests/benchmarking/data.py

@@ -1,4 +1,4 @@
-# Copyright 2016, Optimizely
+# Copyright 2016, 2019, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -3281,6 +3281,7 @@ def dispatch_event(url, params):
 
   return optimizely.Optimizely(datafile, event_dispatcher=NoOpEventDispatcher)
 
+
 optimizely_obj_10_exp = create_optimizely_object(json.dumps(datafiles.get(10)))
 optimizely_obj_25_exp = create_optimizely_object(json.dumps(datafiles.get(25)))
 optimizely_obj_50_exp = create_optimizely_object(json.dumps(datafiles.get(50)))

tests/helpers_tests/test_audience.py

@@ -1,4 +1,4 @@
-# Copyright 2016-2018, Optimizely
+# Copyright 2016-2019, Optimizely
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 # You may obtain a copy of the License at
@@ -14,7 +14,6 @@
 import json
 import mock
 
-from optimizely import entities
 from optimizely import optimizely
 from optimizely.helpers import audience
 from tests import base
@@ -97,7 +96,7 @@ def test_is_user_in_experiment__returns_True__when_condition_tree_evaluator_retu
 
     user_attributes = {'test_attribute': 'test_value_1'}
     experiment = self.project_config.get_experiment_from_key('test_experiment')
-    with mock.patch('optimizely.helpers.condition_tree_evaluator.evaluate', return_value=True) as cond_tree_eval:
+    with mock.patch('optimizely.helpers.condition_tree_evaluator.evaluate', return_value=True):
 
       self.assertStrictTrue(audience.is_user_in_experiment(self.project_config, experiment, user_attributes))
 
@@ -106,19 +105,18 @@ def test_is_user_in_experiment__returns_False__when_condition_tree_evaluator_ret
 
     user_attributes = {'test_attribute': 'test_value_1'}
     experiment = self.project_config.get_experiment_from_key('test_experiment')
-    with mock.patch('optimizely.helpers.condition_tree_evaluator.evaluate', return_value=None) as cond_tree_eval:
+    with mock.patch('optimizely.helpers.condition_tree_evaluator.evaluate', return_value=None):
 
       self.assertStrictFalse(audience.is_user_in_experiment(self.project_config, experiment, user_attributes))
 
-    with mock.patch('optimizely.helpers.condition_tree_evaluator.evaluate', return_value=False) as cond_tree_eval:
+    with mock.patch('optimizely.helpers.condition_tree_evaluator.evaluate', return_value=False):
 
       self.assertStrictFalse(audience.is_user_in_experiment(self.project_config, experiment, user_attributes))
 
   def test_is_user_in_experiment__evaluates_audienceIds(self):
     """ Test that is_user_in_experiment correctly evaluates audience Ids and
         calls custom attribute evaluator for leaf nodes. """
 
-    user_attributes = {'test_attribute': 'test_value_1'}
     experiment = self.project_config.get_experiment_from_key('test_experiment')
     experiment.audienceIds = ['11154', '11159']
     experiment.audienceConditions = None

tests/test_optimizely.py

@@ -372,7 +372,6 @@ def on_custom_event(test_string):
 
   def test_add_invalid_listener(self):
     """ Test adding a invalid listener """
-    not_a_listener = "This is not a listener"
     self.assertEqual(0, len(self.optimizely.notification_center.notifications[enums.NotificationTypes.TRACK]))
 
   def test_add_multi_listener(self):
@@ -456,7 +455,7 @@ def test_track_listener_with_attr(self):
     with mock.patch('optimizely.decision_service.DecisionService.get_variation',
                     return_value=self.project_config.get_variation_from_id(
                       'test_experiment', '111128'
-                    )) as mock_get_variation, \
+                    )), \
       mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch, \
       mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_event_tracked:
       self.optimizely.track('test_event', 'test_user', attributes={'test_attribute': 'test_value'})
@@ -471,7 +470,7 @@ def test_track_listener_with_attr_with_event_tags(self):
     with mock.patch('optimizely.decision_service.DecisionService.get_variation',
                     return_value=self.project_config.get_variation_from_id(
                       'test_experiment', '111128'
-                    )) as mock_get_variation, \
+                    )), \
       mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch, \
       mock.patch('optimizely.notification_center.NotificationCenter.send_notifications') as mock_event_tracked:
       self.optimizely.track('test_event', 'test_user', attributes={'test_attribute': 'test_value'},
@@ -506,7 +505,7 @@ def on_activate(experiment, user_id, attributes, variation, event):
                       mock_variation,
                       decision_service.DECISION_SOURCE_EXPERIMENT
                     )) as mock_decision, \
-      mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event') as mock_dispatch_event, \
+      mock.patch('optimizely.event_dispatcher.EventDispatcher.dispatch_event'), \
       mock.patch('uuid.uuid4', return_value='a68cf1ad-0393-4e18-af87-efe8f01a7c9c'), \
       mock.patch('time.time', return_value=42):
       self.assertTrue(opt_obj.is_feature_enabled('test_feature_in_experiment', 'test_user'))
@@ -2558,7 +2557,7 @@ def test_get_variation__whitelisted_user_forced_bucketing(self):
   def test_get_variation__user_profile__forced_bucketing(self):
     """ Test that the expected forced variation is called if a user profile exists """
     with mock.patch('optimizely.decision_service.DecisionService.get_stored_variation',
-                    return_value=entities.Variation('111128', 'control')) as mock_get_stored_variation:
+                    return_value=entities.Variation('111128', 'control')):
       self.assertTrue(self.optimizely.set_forced_variation('test_experiment', 'test_user', 'variation'))
       self.assertEqual('variation', self.optimizely.get_forced_variation('test_experiment', 'test_user'))
       variation_key = self.optimizely.get_variation('test_experiment',

tox.ini

@@ -1,8 +1,9 @@
-[pep8]
+[flake8]
 # E111 - indentation is not a multiple of four
 # E114 - indentation is not a multiple of four (comment)
 # E121 - continuation line indentation is not a multiple of four
 # E127 - continuation line over-indented for visual indent
-ignore = E111,E114,E121,E127
+# E722 - do not use bare 'except'
+ignore = E111,E114,E121,E127, E722
 exclude = optimizely/lib/pymmh3.py,*virtualenv*
 max-line-length = 120