You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2023/03/10 22:02:15 UTC

[allura] branch master updated (75e6e3c51 -> 13a0670bc)

This is an automated email from the ASF dual-hosted git repository.

brondsem pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git


    from 75e6e3c51 Allow csp_form_actions environ override; more obvious warning if github oauth .ini settings missing
     add c06cce9df [#8502] added ruff as a dependency and updates command in test_syntax
     add de9c2e0b0 [#8502] fixing more code violations
     add 3d53efe85 [#8502] added ruff.toml
     add a9b99fb1f [#8502] updated ruff config and added an extra flag --show-source
     add 26ae660d0 [#8502] regenerated requirements.txt
     new a20a41285 [#8502] ignoring long lines and updating ruff command with --config and --show-source flags
     new cedea1b40 [#8502] remove old unused pylint custom check
     new 13a0670bc [#8502] run ruff all at once, instead of in groups of files

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 Allura/allura/config/middleware.py                 | 30 ++++-----
 Allura/allura/ext/admin/admin_main.py              |  4 +-
 Allura/allura/model/filesystem.py                  |  2 +-
 .../allura/tests/functional/test_neighborhood.py   |  6 +-
 Allura/allura/tests/model/test_timeline.py         |  2 +-
 Allura/allura/tests/test_multifactor.py            |  4 +-
 Allura/allura/tests/test_plugin.py                 |  6 +-
 .../allura/tests/unit/spam/test_stopforumspam.py   |  6 +-
 Allura/setup.py                                    |  2 +-
 AlluraTest/alluratest/pylint_checkers.py           | 73 ----------------------
 AlluraTest/alluratest/test_syntax.py               | 66 ++-----------------
 .../tests/github/functional/test_github.py         |  2 +-
 ForgeTracker/forgetracker/import_support.py        |  4 +-
 requirements.in                                    |  3 +-
 requirements.txt                                   |  4 +-
 ruff.toml                                          | 17 +++++
 scripts/perf/call_count.py                         |  4 +-
 17 files changed, 62 insertions(+), 173 deletions(-)
 delete mode 100644 AlluraTest/alluratest/pylint_checkers.py
 create mode 100644 ruff.toml


[allura] 01/03: [#8502] ignoring long lines and updating ruff command with --config and --show-source flags

Posted by br...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

brondsem pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git

commit a20a412850e06276e5a731ba6280474cb4a6bc89
Author: Guillermo Cruz <gu...@slashdotmedia.com>
AuthorDate: Thu Mar 9 22:34:19 2023 +0000

    [#8502] ignoring long lines and updating ruff command with --config and --show-source flags
---
 AlluraTest/alluratest/test_syntax.py | 14 +++++++-------
 ruff.toml                            |  1 +
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/AlluraTest/alluratest/test_syntax.py b/AlluraTest/alluratest/test_syntax.py
index 7272da81f..da40861de 100644
--- a/AlluraTest/alluratest/test_syntax.py
+++ b/AlluraTest/alluratest/test_syntax.py
@@ -23,7 +23,7 @@ from unittest import SkipTest
 from six.moves import zip_longest
 
 toplevel_dir = os.path.abspath(os.path.dirname(__file__) + "/../..")
-
+BASE_PATH = (toplevel_dir,) #freeze main path
 
 def run(cmd):
     proc = Popen(cmd, shell=True, cwd=toplevel_dir, stdout=PIPE, stderr=PIPE)
@@ -77,11 +77,11 @@ def run_linter(files):
         raise Exception('Custom Allura pylint errors found.')
 
 
-def run_pyflakes(files):
+def run_ruff(files):
     # skip some that aren't critical errors
     files = [f for f in files if '/migrations/' not in f]
-    cmd = "ruff check " + ' '.join(files) + " --show-source"
-    if run(cmd) != 1:
+    cmd = f"ruff check {' '.join(files)}  --config {BASE_PATH[0]}/ruff.toml --show-source"
+    if run(cmd) != 0:
         # print 'Command was: %s' % cmd
         raise Exception('ruff failure, see stdout')
 
@@ -112,9 +112,9 @@ def create_many_lint_methods():
         lint_test_method.__name__ = str(f'test_pylint_{i}')
         setattr(TestLinters, f'test_pylint_{i}', lint_test_method)
 
-        pyflake_test_method = lambda self, these_files=files: run_pyflakes(these_files)
-        pyflake_test_method.__name__ = str(f'test_pyflakes_{i}')
-        setattr(TestLinters, f'test_pyflakes_{i}', pyflake_test_method)
+        pyflake_test_method = lambda self, these_files=files: run_ruff(these_files)
+        pyflake_test_method.__name__ = str(f'test_ruff_{i}')
+        setattr(TestLinters, f'test_ruff_{i}', pyflake_test_method)
 
 
 create_many_lint_methods()
diff --git a/ruff.toml b/ruff.toml
index 7ea3f6a70..1b7a345a0 100644
--- a/ruff.toml
+++ b/ruff.toml
@@ -11,6 +11,7 @@ ignore = [
     'E402', # Module level import not at top of file
     'E731', # Do not assign a lambda expression, use a def
     'E741', # Ambiguous variable name: I,
+    'E501' # Line too long
 ]
 line-length = 119
 


[allura] 02/03: [#8502] remove old unused pylint custom check

Posted by br...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

brondsem pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git

commit cedea1b40fc4efc4a939f4b6828bfde41cb27ab3
Author: Dave Brondsema <db...@slashdotmedia.com>
AuthorDate: Fri Mar 10 16:20:42 2023 -0500

    [#8502] remove old unused pylint custom check
---
 AlluraTest/alluratest/pylint_checkers.py | 73 --------------------------------
 AlluraTest/alluratest/test_syntax.py     | 10 -----
 requirements.in                          |  1 -
 3 files changed, 84 deletions(-)

diff --git a/AlluraTest/alluratest/pylint_checkers.py b/AlluraTest/alluratest/pylint_checkers.py
deleted file mode 100644
index fa2766ff2..000000000
--- a/AlluraTest/alluratest/pylint_checkers.py
+++ /dev/null
@@ -1,73 +0,0 @@
-#       Licensed to the Apache Software Foundation (ASF) under one
-#       or more contributor license agreements.  See the NOTICE file
-#       distributed with this work for additional information
-#       regarding copyright ownership.  The ASF licenses this file
-#       to you 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
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#       Unless required by applicable law or agreed to in writing,
-#       software distributed under the License is distributed on an
-#       "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-#       KIND, either express or implied.  See the License for the
-#       specific language governing permissions and limitations
-#       under the License.
-
-import astroid
-
-from pylint.checkers import BaseChecker, utils
-from pylint.interfaces import IAstroidChecker
-
-
-def register(linter):
-    linter.register_checker(ExposedAPIHasKwargs(linter))
-
-
-# FIXME?
-BASE_ID = 76  # taken from https://github.com/edx/edx-lint/tree/master/edx_lint/pylint
-
-class ExposedAPIHasKwargs(BaseChecker):
-
-    __implements__ = (IAstroidChecker,)
-
-    name = 'exposed-api-needs-kwargs'
-
-    MESSAGE_ID = name
-    msgs = {
-        'E%d10' % BASE_ID: (
-            "@expose'd method %s() looks like an API and needs **kwargs",
-            MESSAGE_ID,
-            "@expose'd API methods should have **kwargs to avoid an error when accessed with ?access_token=XYZ",
-        ),
-    }
-
-    @utils.check_messages(MESSAGE_ID)
-    def visit_function(self, node):
-        # special TurboGears method name, doesn't need **kw
-        if node.name == '_lookup':
-            return
-
-        # Assume @expose('json:') means its an API endpoint.  Not a perfect assumption:
-        #  - could be an API endpoint used only for PUT/POST with no json result)
-        #  - there are non-API endpoints that return json (for ajax)
-        has_json_expose = False
-        if node.decorators:
-            for dec in node.decorators.nodes:
-                if hasattr(dec, 'func'):  # otherwise its a @deco without parens
-                    if getattr(dec.func, 'name', '') == 'expose':
-                        for arg in dec.args:
-                            if getattr(arg, 'value', '') in ('json', 'json:'):
-                                has_json_expose = True
-
-        if not has_json_expose:
-            return
-
-        if node.args.kwarg:
-            return
-
-        if not self.linter.is_message_enabled(self.MESSAGE_ID, line=node.fromlineno):
-            return
-
-        self.add_message(self.MESSAGE_ID, args=node.name, node=node)
diff --git a/AlluraTest/alluratest/test_syntax.py b/AlluraTest/alluratest/test_syntax.py
index da40861de..6bf68f3f1 100644
--- a/AlluraTest/alluratest/test_syntax.py
+++ b/AlluraTest/alluratest/test_syntax.py
@@ -71,12 +71,6 @@ def test_no_tabs():
         raise Exception('These should not use tab chars')
 
 
-def run_linter(files):
-    raise SkipTest('pylint see [#8346]')
-    if run('pylint -E --disable=all --enable=exposed-api-needs-kwargs --load-plugins alluratest.pylint_checkers {}'.format(' '.join(files))) != 0:
-        raise Exception('Custom Allura pylint errors found.')
-
-
 def run_ruff(files):
     # skip some that aren't critical errors
     files = [f for f in files if '/migrations/' not in f]
@@ -108,10 +102,6 @@ def create_many_lint_methods():
         if not files:
             continue
 
-        lint_test_method = lambda self, these_files=files: run_linter(these_files)
-        lint_test_method.__name__ = str(f'test_pylint_{i}')
-        setattr(TestLinters, f'test_pylint_{i}', lint_test_method)
-
         pyflake_test_method = lambda self, these_files=files: run_ruff(these_files)
         pyflake_test_method.__name__ = str(f'test_ruff_{i}')
         setattr(TestLinters, f'test_ruff_{i}', pyflake_test_method)
diff --git a/requirements.in b/requirements.in
index e209b3487..0644bdfdb 100644
--- a/requirements.in
+++ b/requirements.in
@@ -50,7 +50,6 @@ wrapt
 # testing
 mock
 ruff
-#pylint -- disabled due to [#8346]  (also requires diff versions on py2 vs 3, including transitive deps which gets tricky with pip-compile)
 testfixtures
 WebTest
 pytest


[allura] 03/03: [#8502] run ruff all at once, instead of in groups of files

Posted by br...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

brondsem pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/allura.git

commit 13a0670bcb84525dd84d03afce6a52e7d5d4193e
Author: Dave Brondsema <db...@slashdotmedia.com>
AuthorDate: Fri Mar 10 16:51:21 2023 -0500

    [#8502] run ruff all at once, instead of in groups of files
---
 AlluraTest/alluratest/test_syntax.py | 44 +++---------------------------------
 scripts/perf/call_count.py           |  4 ++--
 2 files changed, 5 insertions(+), 43 deletions(-)

diff --git a/AlluraTest/alluratest/test_syntax.py b/AlluraTest/alluratest/test_syntax.py
index 6bf68f3f1..39eef5302 100644
--- a/AlluraTest/alluratest/test_syntax.py
+++ b/AlluraTest/alluratest/test_syntax.py
@@ -37,13 +37,6 @@ def run(cmd):
 find_py = r"find Allura Forge* -not -path '*/\.*' -name '*.py'"
 
 
-# a recipe from itertools doc
-def grouper(n, iterable, fillvalue=None):
-    "grouper(3, 'ABCDEFG', 'x') --> ABC DEF Gxx"
-    args = [iter(iterable)] * n
-    return zip_longest(fillvalue=fillvalue, *args)
-
-
 def test_no_local_tz_functions():
     if run(find_py + r" | xargs grep '\.now(' ") not in [1, 123]:
         raise Exception("These should use .utcnow()")
@@ -62,7 +55,7 @@ def test_no_prints():
         '/scripts/',
         'ForgeSVN/setup.py',
     ]
-    if run(find_py + " | grep -v '" + "' | grep -v '".join(skips) + "' | xargs grep -v '^ *#' | egrep -n '\bprint\(' | grep -E -v '(pprint|#pragma: ?printok)' ") != 1:
+    if run(find_py + " | grep -v '" + "' | grep -v '".join(skips) + r"' | xargs grep -v '^ *#' | egrep -n '\bprint\(' | grep -E -v '(pprint|#pragma: ?printok)' ") != 1:
         raise Exception("These should use logging instead of print")
 
 
@@ -71,40 +64,9 @@ def test_no_tabs():
         raise Exception('These should not use tab chars')
 
 
-def run_ruff(files):
-    # skip some that aren't critical errors
-    files = [f for f in files if '/migrations/' not in f]
-    cmd = f"ruff check {' '.join(files)}  --config {BASE_PATH[0]}/ruff.toml --show-source"
+def test_ruff():
+    cmd = f"ruff check . --config {BASE_PATH[0]}/ruff.toml --show-source"
     if run(cmd) != 0:
         # print 'Command was: %s' % cmd
         raise Exception('ruff failure, see stdout')
 
-
-class TestLinters:
-    # this will get populated dynamically with test methods, see below
-    pass
-
-
-# Dynamically generate many test methods, to run pylint & ruff commands in separate batches
-# Can't use http://nose.readthedocs.io/en/latest/writing_tests.html#test-generators because nose doesn't run
-# those in parallel
-def create_many_lint_methods():
-    proc = Popen(find_py, shell=True, cwd=toplevel_dir, stdout=PIPE, stderr=PIPE)
-    (find_stdout, stderr) = proc.communicate()
-    find_stdout = find_stdout.decode('utf-8')
-    stderr = stderr.decode('utf-8')
-    sys.stderr.write(stderr)
-    assert proc.returncode == 0, proc.returncode
-    py_files = find_stdout.split('\n')
-
-    for i, files in enumerate(grouper(40, py_files)):
-        files = [_f for _f in files if _f]
-        if not files:
-            continue
-
-        pyflake_test_method = lambda self, these_files=files: run_ruff(these_files)
-        pyflake_test_method.__name__ = str(f'test_ruff_{i}')
-        setattr(TestLinters, f'test_ruff_{i}', pyflake_test_method)
-
-
-create_many_lint_methods()
diff --git a/scripts/perf/call_count.py b/scripts/perf/call_count.py
index 4c567b7df..32cf5f3d8 100755
--- a/scripts/perf/call_count.py
+++ b/scripts/perf/call_count.py
@@ -70,7 +70,7 @@ def main(args):
                         debug_html=args.debug_html)
     print(json.dumps(counts))
     write_csv(counts, args.id, args.data_file)
-    test.teardown_method(method)
+    test.teardown_method()
 
 
 def setup(test):
@@ -79,7 +79,7 @@ def setup(test):
                                   'stats.debug_line_length': 1000,
                                   }), \
             patch('timermiddleware.log.isEnabledFor', return_value=True):  # can't set this via logging configuration since setUp() will load a logging config and then start using it before we have a good place to tweak it
-        test.setup_method(method)
+        test.setup_method()
 
     tmw_log = logging.getLogger('timermiddleware')
     tmw_log.disabled = 0  # gets disabled when .ini file is loaded; dumb.