Skip to content

Commit

Permalink
Nacho/fix rolling build (#9)
Browse files Browse the repository at this point in the history
* Reformat a bit the help, no real changes

* Use black python API to fetch the list of checked files

* Ignore colcon build artifacts

* Use single quotes per ROS Standards

Yes, this package does not pass `black` format anymore.

Info:
  - https://docs.ros.org/en/rolling/The-ROS2-Project/Contributing/Code-Style-Language-Versions.html

* Fix imports

* This just got messy

* Add ruff as pre-commit for ROS style on black repo

* Hack the CI to make it ROS-happy

* Move inside

* Use isort --profile=google for making ROS folks happy

* Already defaults

* remove ruff.toml

* Update pre-commit

* Fix ci

* Add flake8 with (unlisted) plugins to avoid this pain in future
  • Loading branch information
nachovizzo authored Jan 19, 2024
1 parent 90f0c3e commit 60d353d
Show file tree
Hide file tree
Showing 13 changed files with 127 additions and 102 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ jobs:
- name: Build pip package
run: python -m pip install --verbose ./ament_black
- name: Test installation
run: ament_black .
# black does not comply with ROS standards, thefore will always fail to check the style
run: ament_black . || echo
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
build/
install/
log/
# Created by https://www.toptal.com/developers/gitignore/api/python
# Edit at https://www.toptal.com/developers/gitignore?templates=python

Expand Down
26 changes: 20 additions & 6 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,31 @@ repos:
- id: mixed-line-ending
- id: trailing-whitespace

- repo: https://github.com/psf/black
rev: 22.3.0
hooks:
- id: black

- repo: https://github.com/cheshirekow/cmake-format-precommit
rev: v0.6.10
hooks:
- id: cmake-format

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.1.13
hooks:
- id: ruff-format

- repo: https://github.com/pycqa/isort
rev: 5.12.0
rev: 5.13.2
hooks:
- id: isort
args: [--profile, google]

- repo: https://github.com/pycqa/flake8
rev: 4.0.1
hooks:
- id: flake8
args: [--config, ./ament_black/.flake8]
additional_dependencies:
[
flake8-builtins,
flake8-comprehensions,
flake8-docstrings,
flake8-import-order,
]
6 changes: 6 additions & 0 deletions ament_black/.flake8
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[flake8]
extend-ignore = B902,C816,D100,D101,D102,D103,D104,D105,D106,D107,D203,D212,D404,I202
import-order-style = google
max-line-length = 99
show-source = true
statistics = true
94 changes: 45 additions & 49 deletions ament_black/ament_black/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,17 @@
import sys
import tempfile
import time
from xml.sax.saxutils import escape, quoteattr
from xml.sax.saxutils import escape
from xml.sax.saxutils import quoteattr

import click
from black import get_sources
from black import main as black
from black import re_compile_maybe_verbose
from black.concurrency import maybe_install_uvloop
from black.const import DEFAULT_EXCLUDES, DEFAULT_INCLUDES
from black.const import DEFAULT_EXCLUDES
from black.const import DEFAULT_INCLUDES
from black.report import Report
import click
from unidiff import PatchSet


Expand All @@ -43,30 +45,30 @@ def patched_black(*args, **kwargs) -> None:

def main(argv=sys.argv[1:]):
parser = argparse.ArgumentParser(
description="Check code style using black.",
description='Check code style using black.',
formatter_class=argparse.ArgumentDefaultsHelpFormatter,
)
parser.add_argument(
"paths",
nargs="*",
'paths',
nargs='*',
default=[os.curdir],
help="The files or directories to check. this argument is directly passed to black",
help='The files or directories to check. this argument is directly passed to black',
)
parser.add_argument(
"--config",
metavar="path",
'--config',
metavar='path',
default=None,
dest="config_file",
help="The config file",
dest='config_file',
help='The config file',
)
parser.add_argument(
"--reformat",
action="store_true",
help="Reformat the files in place",
'--reformat',
action='store_true',
help='Reformat the files in place',
)
parser.add_argument(
"--xunit-file",
help="Generate a xunit compliant XML file",
'--xunit-file',
help='Generate a xunit compliant XML file',
)
args = parser.parse_args(argv)

Expand All @@ -86,7 +88,7 @@ def main(argv=sys.argv[1:]):
extend_exclude=None,
force_exclude=None,
report=Report(),
stdin_filename="",
stdin_filename='',
)
checked_files = [str(path) for path in sources]

Expand All @@ -96,14 +98,14 @@ def main(argv=sys.argv[1:]):
# invoke black
black_args_withouth_path = []
if args.config_file is not None:
black_args_withouth_path.extend(["--config", args.config_file])
black_args_withouth_path.extend(['--config', args.config_file])
black_args = black_args_withouth_path.copy()
black_args.extend(args.paths)

with tempfile.NamedTemporaryFile("w") as diff:
with tempfile.NamedTemporaryFile('w') as diff:
with contextlib.redirect_stdout(diff):
patched_black([*black_args, "--diff"], standalone_mode=False)
with open(diff.name, "r") as file:
patched_black([*black_args, '--diff'], standalone_mode=False)
with open(diff.name, 'r') as file:
output = file.read()

# output errors
Expand All @@ -127,11 +129,11 @@ def main(argv=sys.argv[1:]):
file_count = sum(1 if report[k] else 0 for k in report.keys())
replacement_count = sum(len(r) for r in report.values())
if not file_count:
print("No problems found")
print('No problems found')
rc = 0
else:
print(
"%d files with %d code style divergences" % (file_count, replacement_count),
'%d files with %d code style divergences' % (file_count, replacement_count),
file=sys.stderr,
)
rc = 1
Expand All @@ -140,51 +142,49 @@ def main(argv=sys.argv[1:]):
if args.xunit_file:
folder_name = os.path.basename(os.path.dirname(args.xunit_file))
file_name = os.path.basename(args.xunit_file)
suffix = ".xml"
suffix = '.xml'
if file_name.endswith(suffix):
file_name = file_name.split(suffix)[0]
testname = "%s.%s" % (folder_name, file_name)
testname = '%s.%s' % (folder_name, file_name)

xml = get_xunit_content(
report, testname, time.time() - start_time, checked_files
)
xml = get_xunit_content(report, testname, time.time() - start_time, checked_files)
path = os.path.dirname(os.path.abspath(args.xunit_file))
if not os.path.exists(path):
os.makedirs(path)
with open(args.xunit_file, "w") as f:
with open(args.xunit_file, 'w') as f:
f.write(xml)

return rc


def find_index_of_line_start(data, offset):
index_1 = data.rfind("\n", 0, offset) + 1
index_2 = data.rfind("\r", 0, offset) + 1
index_1 = data.rfind('\n', 0, offset) + 1
index_2 = data.rfind('\r', 0, offset) + 1
return max(index_1, index_2)


def find_index_of_line_end(data, offset):
index_1 = data.find("\n", offset)
index_1 = data.find('\n', offset)
if index_1 == -1:
index_1 = len(data)
index_2 = data.find("\r", offset)
index_2 = data.find('\r', offset)
if index_2 == -1:
index_2 = len(data)
return min(index_1, index_2)


def get_line_number(data, offset):
return data[0:offset].count("\n") + data[0:offset].count("\r") + 1
return data[0:offset].count('\n') + data[0:offset].count('\r') + 1


def get_xunit_content(report, testname, elapsed, checked_files):
test_count = sum(max(len(r), 1) for r in report.values())
error_count = sum(len(r) for r in report.values())
data = {
"testname": testname,
"test_count": test_count,
"error_count": error_count,
"time": "%.3f" % round(elapsed, 3),
'testname': testname,
'test_count': test_count,
'error_count': error_count,
'time': '%.3f' % round(elapsed, 3),
}
xml = (
"""<?xml version="1.0" encoding="UTF-8"?>
Expand All @@ -205,11 +205,9 @@ def get_xunit_content(report, testname, elapsed, checked_files):
if hunks:
for hunk in hunks:
data = {
"quoted_location": quoteattr(
"%s:%d" % (filename, hunk.source_start)
),
"testname": testname,
"quoted_message": quoteattr(str(hunk)),
'quoted_location': quoteattr('%s:%d' % (filename, hunk.source_start)),
'testname': testname,
'quoted_message': quoteattr(str(hunk)),
}
xml += (
""" <testcase
Expand All @@ -224,7 +222,7 @@ def get_xunit_content(report, testname, elapsed, checked_files):

else:
# if there are no replacements report a single successful test
data = {"quoted_location": quoteattr(filename), "testname": testname}
data = {'quoted_location': quoteattr(filename), 'testname': testname}
xml += (
""" <testcase
name=%(quoted_location)s
Expand All @@ -234,18 +232,16 @@ def get_xunit_content(report, testname, elapsed, checked_files):
)

# output list of checked files
data = {
"checked_files": escape("".join(["\n* %s" % r for r in sorted(checked_files)]))
}
data = {'checked_files': escape(''.join(['\n* %s' % r for r in sorted(checked_files)]))}
xml += (
""" <system-out>Checked files:%(checked_files)s</system-out>
"""
% data
)

xml += "</testsuite>\n"
xml += '</testsuite>\n'
return xml


if __name__ == "__main__":
if __name__ == '__main__':
sys.exit(main())
4 changes: 2 additions & 2 deletions ament_black/ament_black/pytest_marker.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@

def pytest_configure(config):
config.addinivalue_line(
"markers",
"ament_black: marks tests checking for ament-black CLI interface",
'markers',
'ament_black: marks tests checking for ament-black CLI interface',
)
8 changes: 8 additions & 0 deletions ament_black/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[tool.ruff]
line-length = 99

[tool.ruff.format]
quote-style = "single"

[tool.isort]
profile = "google"
55 changes: 28 additions & 27 deletions ament_black/setup.py
Original file line number Diff line number Diff line change
@@ -1,45 +1,46 @@
from setuptools import find_packages, setup
from setuptools import find_packages
from setuptools import setup

package_name = "ament_black"
package_name = 'ament_black'

setup(
name=package_name,
version="0.2.3",
packages=find_packages(exclude=["test"]),
version='0.2.3',
packages=find_packages(exclude=['test']),
data_files=[
("share/" + package_name, ["package.xml"]),
("share/ament_index/resource_index/packages", ["resource/" + package_name]),
('share/' + package_name, ['package.xml']),
('share/ament_index/resource_index/packages', ['resource/' + package_name]),
],
install_requires=[
"black==21.12b0",
"setuptools>=56",
"unidiff>=0.5",
"uvloop==0.17.0",
'black==21.12b0',
'setuptools>=56',
'unidiff>=0.5',
'uvloop==0.17.0',
],
zip_safe=False,
author="Tyler Weaver",
author_email="tylerjw@gmail.com",
maintainer="Ignacio Vizzo",
maintainer_email="ignacio.vizzo@dexory.com",
url="https://github.com/botsandus/ament_black",
download_url="https://github.com/botsandus/ament_black/releases",
keywords=["ROS"],
author='Tyler Weaver',
author_email='tylerjw@gmail.com',
maintainer='Ignacio Vizzo',
maintainer_email='ignacio.vizzo@dexory.com',
url='https://github.com/botsandus/ament_black',
download_url='https://github.com/botsandus/ament_black/releases',
keywords=['ROS'],
classifiers=[
"Intended Audience :: Developers",
"License :: OSI Approved :: Apache Software License",
"Programming Language :: Python",
"Topic :: Software Development",
'Intended Audience :: Developers',
'License :: OSI Approved :: Apache Software License',
'Programming Language :: Python',
'Topic :: Software Development',
],
description="Check Python code style using black.",
description='Check Python code style using black.',
long_description="""\
The ability to check code against style conventions using black
and generate xUnit test result files.""",
license="Apache License, Version 2.0, BSD",
tests_require=["pytest"],
license='Apache License, Version 2.0, BSD',
tests_require=['pytest'],
entry_points={
"console_scripts": ["ament_black = ament_black.main:main"],
"pytest11": [
"ament_black = ament_black.pytest_marker",
'console_scripts': ['ament_black = ament_black.main:main'],
'pytest11': [
'ament_black = ament_black.pytest_marker',
],
},
)
4 changes: 2 additions & 2 deletions ament_black/test/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
@pytest.mark.ament_black
@pytest.mark.linter
def test_ament_black():
rc = os.system("ament_black --help")
assert rc == 0, "ament-black python package not properly installed"
rc = os.system('ament_black --help')
assert rc == 0, 'ament-black python package not properly installed'
10 changes: 4 additions & 6 deletions ament_black/test/test_copyright.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,14 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import pytest
from ament_copyright.main import main
import pytest


# Remove the `skip` decorator once the source file(s) have a copyright header
@pytest.mark.skip(
reason="No copyright header has been placed in the generated source file."
)
@pytest.mark.skip(reason='No copyright header has been placed in the generated source file.')
@pytest.mark.copyright
@pytest.mark.linter
def test_copyright():
rc = main(argv=[".", "test"])
assert rc == 0, "Found errors"
rc = main(argv=['.', 'test'])
assert rc == 0, 'Found errors'
Loading

0 comments on commit 60d353d

Please sign in to comment.