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 2024/01/03 19:58:52 UTC
(allura) 02/04: [#8532] add Bugbear ruff checks, other fixes
This is an automated email from the ASF dual-hosted git repository.
brondsem pushed a commit to branch db/8532
in repository https://gitbox.apache.org/repos/asf/allura.git
commit ca17f10ce3082b0d7b6285fd655bc91a772b9d6f
Author: Dave Brondsema <db...@slashdotmedia.com>
AuthorDate: Tue Jan 2 17:42:29 2024 -0500
[#8532] add Bugbear ruff checks, other fixes
---
Allura/allura/controllers/auth.py | 4 +--
Allura/allura/lib/custom_middleware.py | 5 ++--
Allura/allura/lib/macro.py | 2 +-
Allura/allura/model/timeline.py | 2 +-
Allura/allura/tasks/repo_tasks.py | 2 +-
Allura/allura/tests/model/test_artifact.py | 2 +-
Allura/allura/tests/model/test_auth.py | 2 +-
Allura/allura/tests/model/test_project.py | 6 ++---
.../allura/tests/unit/test_package_path_loader.py | 2 +-
ForgeDiscussion/forgediscussion/model/forum.py | 2 +-
.../forgediscussion/tests/functional/test_forum.py | 2 +-
ForgeGit/forgegit/tests/model/test_repository.py | 10 ++++----
ForgeSVN/forgesvn/tests/model/test_repository.py | 6 ++---
ForgeTracker/forgetracker/model/ticket.py | 2 +-
ruff.toml | 30 +++++++++++++++-------
15 files changed, 46 insertions(+), 33 deletions(-)
diff --git a/Allura/allura/controllers/auth.py b/Allura/allura/controllers/auth.py
index 7cd0189c2..1d11394f5 100644
--- a/Allura/allura/controllers/auth.py
+++ b/Allura/allura/controllers/auth.py
@@ -452,8 +452,8 @@ class AuthController(BaseController):
return '.'.join(reversed(parts))
repos = []
- for p in user.my_projects():
- for p in [p] + p.direct_subprojects:
+ for up in user.my_projects():
+ for p in [up] + up.direct_subprojects:
for app in p.app_configs:
if not issubclass(g.entry_points["tool"][app.tool_name], RepositoryApp):
continue
diff --git a/Allura/allura/lib/custom_middleware.py b/Allura/allura/lib/custom_middleware.py
index e74594aa0..d9ca652fb 100644
--- a/Allura/allura/lib/custom_middleware.py
+++ b/Allura/allura/lib/custom_middleware.py
@@ -286,7 +286,8 @@ class SetRequestHostFromConfig:
# 'HTTP_X_FORWARDED_PROTO' == 'https'
req = Request(environ)
try:
- req.params # check for malformed unicode or POSTs, this is the first middleware that might trip over it.
+ # check for malformed unicode or POSTs, this is the first middleware that might trip over it:
+ req.params # noqa: B018
resp = self.app
except (UnicodeError, ValueError):
resp = exc.HTTPBadRequest()
@@ -456,7 +457,7 @@ class MingTaskSessionSetupMiddleware:
def __call__(self, environ, start_response):
# this is sufficient to ensure an ODM session is always established
- session(M.MonQTask).impl
+ session(M.MonQTask).impl # noqa: B018
return self.app(environ, start_response)
diff --git a/Allura/allura/lib/macro.py b/Allura/allura/lib/macro.py
index 20d8fb244..7d0aba893 100644
--- a/Allura/allura/lib/macro.py
+++ b/Allura/allura/lib/macro.py
@@ -67,7 +67,7 @@ class macro:
Decorator to declare a function is a custom markdown macro
"""
- def __init__(self, context: str = None, cacheable: bool = False):
+ def __init__(self, context: str | None = None, cacheable: bool = False):
"""
:param context: either "neighborhood-wiki" or "userproject-wiki" to limit the macro to be used in those contexts
:param cacheable: indicates if its ok to cache the macro's output permanently
diff --git a/Allura/allura/model/timeline.py b/Allura/allura/model/timeline.py
index 85d28eda0..9048b6c1e 100644
--- a/Allura/allura/model/timeline.py
+++ b/Allura/allura/model/timeline.py
@@ -43,7 +43,7 @@ class Director(ActivityDirector):
"""
- def create_activity(self, actor, verb, obj, target=None,
+ def create_activity(self, actor: ActivityNode, verb: str, obj: ActivityObject, target=None,
related_nodes=None, tags=None):
if c.project and c.project.notifications_disabled:
return
diff --git a/Allura/allura/tasks/repo_tasks.py b/Allura/allura/tasks/repo_tasks.py
index 296358fde..b7185bc39 100644
--- a/Allura/allura/tasks/repo_tasks.py
+++ b/Allura/allura/tasks/repo_tasks.py
@@ -179,7 +179,7 @@ def can_merge(merge_request_id):
def determine_mr_commits(merge_request_id):
from allura import model as M
mr = M.MergeRequest.query.get(_id=merge_request_id)
- mr.commits # build & cache the commits
+ mr.commits # noqa: B018. build & cache the commits
@task
diff --git a/Allura/allura/tests/model/test_artifact.py b/Allura/allura/tests/model/test_artifact.py
index aed8add97..9eb2c8b4e 100644
--- a/Allura/allura/tests/model/test_artifact.py
+++ b/Allura/allura/tests/model/test_artifact.py
@@ -186,7 +186,7 @@ class TestArtifact:
pg.delete()
ThreadLocalODMSession.flush_all()
M.MonQTask.run_ready()
- WM.PageHistory.query.find({'artifact_id': _id}).count() == 0
+ assert WM.PageHistory.query.find({'artifact_id': _id}).count() == 0
# history should be deleted from solr
assert g.solr.search('id:' + ph_index).hits == 0
diff --git a/Allura/allura/tests/model/test_auth.py b/Allura/allura/tests/model/test_auth.py
index 570bea349..11cd00cb4 100644
--- a/Allura/allura/tests/model/test_auth.py
+++ b/Allura/allura/tests/model/test_auth.py
@@ -236,7 +236,7 @@ class TestAuth:
roles = M.ProjectRole.query.find({'_id': {'$in': roles_ids}})
for pr in roles:
assert pr.display()
- pr.special
+ assert pr.special
assert pr.user in (c.user, None, M.User.anonymous())
def test_default_project_roles(self):
diff --git a/Allura/allura/tests/model/test_project.py b/Allura/allura/tests/model/test_project.py
index 3d44419c8..b5850b87c 100644
--- a/Allura/allura/tests/model/test_project.py
+++ b/Allura/allura/tests/model/test_project.py
@@ -41,12 +41,12 @@ class TestProjectModel:
setup_global_objects()
def test_project(self):
- assert type(c.project.sidebar_menu()) == list
+ assert isinstance(c.project.sidebar_menu(), list)
assert c.project.script_name in c.project.url()
old_proj = c.project
h.set_context('test/sub1', neighborhood='Projects')
- assert type(c.project.sidebar_menu()) == list
- assert type(c.project.sitemap()) == list
+ assert isinstance(c.project.sidebar_menu(), list)
+ assert isinstance(c.project.sitemap(), list)
assert c.project.sitemap()[1].label == 'Admin'
assert old_proj in list(c.project.parent_iter())
h.set_context('test', 'wiki', neighborhood='Projects')
diff --git a/Allura/allura/tests/unit/test_package_path_loader.py b/Allura/allura/tests/unit/test_package_path_loader.py
index 44bb6fbfe..192eed615 100644
--- a/Allura/allura/tests/unit/test_package_path_loader.py
+++ b/Allura/allura/tests/unit/test_package_path_loader.py
@@ -50,7 +50,7 @@ class TestPackagePathLoader(TestCase):
['ep2', 'path:eps.ep2'],
['allura', '/'],
]
- assert type(paths[0]) == list
+ assert isinstance(paths[0], list)
assert resource_filename.call_args_list == [
mock.call('eps.ep0', ''),
mock.call('eps.ep1', ''),
diff --git a/ForgeDiscussion/forgediscussion/model/forum.py b/ForgeDiscussion/forgediscussion/model/forum.py
index 06c0e16cb..3e0be4ae7 100644
--- a/ForgeDiscussion/forgediscussion/model/forum.py
+++ b/ForgeDiscussion/forgediscussion/model/forum.py
@@ -163,7 +163,7 @@ class ForumThread(M.Thread):
discussion = RelationProperty(Forum)
posts = RelationProperty('ForumPost', via='thread_id')
- first_post = RelationProperty('ForumPost', via='first_post_id')
+ first_post: 'ForumPost' = RelationProperty('ForumPost', via='first_post_id')
@property
def type_name(self):
diff --git a/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py b/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
index 18e2a3f93..b06aa2026 100644
--- a/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
+++ b/ForgeDiscussion/forgediscussion/tests/functional/test_forum.py
@@ -496,7 +496,7 @@ class TestForum(TestController):
self.test_posting()
# second should fail
- with pytest.raises(Exception):
+ with pytest.raises(AssertionError, match='Message posted'):
self.test_posting()
def test_notifications_escaping(self):
diff --git a/ForgeGit/forgegit/tests/model/test_repository.py b/ForgeGit/forgegit/tests/model/test_repository.py
index ce2118f7e..b8d58ae01 100644
--- a/ForgeGit/forgegit/tests/model/test_repository.py
+++ b/ForgeGit/forgegit/tests/model/test_repository.py
@@ -68,8 +68,8 @@ class TestNewGit(unittest.TestCase):
def test_commit(self):
assert self.rev.primary() is self.rev
assert self.rev.index_id().startswith('allura/model/repo/Commit#')
- self.rev.author_url
- self.rev.committer_url
+ assert self.rev.author_url is None
+ assert self.rev.committer_url is None
assert self.rev.tree._id == self.rev.tree_id
assert self.rev.summary == self.rev.message.splitlines()[0]
assert self.rev.shorthand_id() == '[1e146e]'
@@ -90,7 +90,7 @@ class TestNewGit(unittest.TestCase):
'/p/test/src-git/ci/'
'1e146e67985dcd71c74de79613719bef7bddca4a/'
'tree/')
- self.rev.tree.by_name['README']
+ assert self.rev.tree.by_name['README']
assert self.rev.tree.is_blob('README') is True
ThreadLocalODMSession.close_all()
c.app = None
@@ -789,8 +789,8 @@ By Dave Brondsema''' in text_body
@mock.patch('forgegit.model.git_repo.git', autospec=True)
def test_merge_raise_exception(self, git, shutil, tempfile):
self.repo._impl._git.git = mock.Mock()
- git.Repo.clone_from.side_effect = Exception
- with self.assertRaises(Exception):
+ git.Repo.clone_from.side_effect = ConnectionError
+ with self.assertRaises(ConnectionError):
self.repo.merge(mock.Mock())
assert shutil.rmtree.called
diff --git a/ForgeSVN/forgesvn/tests/model/test_repository.py b/ForgeSVN/forgesvn/tests/model/test_repository.py
index 79b6e75e5..5165c6ced 100644
--- a/ForgeSVN/forgesvn/tests/model/test_repository.py
+++ b/ForgeSVN/forgesvn/tests/model/test_repository.py
@@ -77,8 +77,8 @@ class TestNewRepo(unittest.TestCase):
latest_rev = 7
assert self.rev.primary() is self.rev
assert self.rev.index_id().startswith('allura/model/repo/Commit#')
- self.rev.author_url
- self.rev.committer_url
+ assert self.rev.author_url is None
+ assert self.rev.committer_url is None
assert self.rev.tree._id == self.rev.tree_id
assert self.rev.shorthand_id() == f'[r{latest_rev}]'
assert self.rev.symbolic_ids == ([], [])
@@ -89,7 +89,7 @@ class TestNewRepo(unittest.TestCase):
assert self.rev.tree.readme() == ('README', 'This is readme\nAnother Line\n')
assert self.rev.tree.path() == '/'
assert self.rev.tree.url() == f'/p/test/src/{latest_rev}/tree/'
- self.rev.tree.by_name['README']
+ assert self.rev.tree.by_name['README']
assert self.rev.tree.is_blob('README') is True
assert self.rev.tree['a']['b']['c'].ls() == []
self.assertRaises(KeyError, lambda: self.rev.tree['a']['b']['d'])
diff --git a/ForgeTracker/forgetracker/model/ticket.py b/ForgeTracker/forgetracker/model/ticket.py
index 3d358842f..bc6910df8 100644
--- a/ForgeTracker/forgetracker/model/ticket.py
+++ b/ForgeTracker/forgetracker/model/ticket.py
@@ -431,7 +431,7 @@ class Globals(MappedClass):
getattr(ticket, k))
setattr(ticket, k, v)
for k, v in sorted(custom_values.items()):
- def cf_val(cf):
+ def cf_val(cf, ticket=ticket):
return ticket.get_custom_user(cf.name) \
if cf.type == 'user' \
else ticket.custom_fields.get(cf.name)
diff --git a/ruff.toml b/ruff.toml
index 88b6cea3f..f7453d735 100644
--- a/ruff.toml
+++ b/ruff.toml
@@ -15,8 +15,20 @@
# specific language governing permissions and limitations
# under the License.
-per-file-ignores = {'__init__.py' = ['F403']}
-select = ['E', 'F', 'RUF100', 'W6', 'ISC001']
+line-length = 119
+
+
+select = [
+ # all flake8 & pep8 (except 'ignore' below)
+ "F",
+ "E",
+ "W6", # some deprecation checks
+ "RUF100", # unncessary noqa
+ "RUF013", # implicit Optional (=None)
+ "ISC001", # NIC001 in flake8 codes
+ "B",
+]
+
ignore = [
'F401', # Imported but unused,
'F811', # Redefinition of unused
@@ -25,12 +37,12 @@ ignore = [
'E731', # Do not assign a lambda expression, use a def
'E741', # Ambiguous variable name: I,
'E501', # Line too long
- # REMOVE THESE AND FIX THE ISSUES
- 'F541', # f-string without any placeholders
- 'E401', # Multiple imports on one line
- 'E721', # Do not compare types, use `isinstance()`
- 'E713', # Test for membership should be `not in`
- 'E701' # Multiple statements on one line (colon)
+ 'B006', # Do not use mutable data structures for argument defaults
+ 'B007', # Loop control variable not used within the loop body
+ 'B904', # use raise from
]
-line-length = 119
+
+[per-file-ignores]
+'__init__.py' = ['F403'] # import *
+'**/{alluratest,tests}/*' = ['B011'] # assert False