Skip to content

Commit

Permalink
test_runner: replace black with ruff format (#6268)
Browse files Browse the repository at this point in the history
## Problem

`black` is slow sometimes, we can replace it with `ruff format` (a new
feature in 0.1.2 [0]), which produces pretty similar to black style [1].

On my local machine (MacBook M1 Pro 16GB):
```
# `black` on main
$ hyperfine "BLACK_CACHE_DIR=/dev/null poetry run black ."
Benchmark 1: BLACK_CACHE_DIR=/dev/null poetry run black .
  Time (mean ± σ):      3.131 s ±  0.090 s    [User: 5.194 s, System: 0.859 s]
  Range (min … max):    3.047 s …  3.354 s    10 runs
```
```
# `ruff format` on the current PR
$ hyperfine "RUFF_NO_CACHE=true poetry run ruff format"      
Benchmark 1: RUFF_NO_CACHE=true poetry run ruff format
  Time (mean ± σ):     300.7 ms ±  50.2 ms    [User: 259.5 ms, System: 76.1 ms]
  Range (min … max):   267.5 ms … 420.2 ms    10 runs
```

## Summary of changes
- Replace `black` with `ruff format` everywhere

- [0] https://docs.astral.sh/ruff/formatter/
- [1] https://docs.astral.sh/ruff/formatter/#black-compatibility
  • Loading branch information
bayandin authored Jan 5, 2024
1 parent 3c560d2 commit 7de829e
Show file tree
Hide file tree
Showing 19 changed files with 79 additions and 185 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ jobs:
- name: Install Python deps
run: ./scripts/pysync

- name: Run ruff to ensure code format
run: poetry run ruff .
- name: Run `ruff check` to ensure code format
run: poetry run ruff check .

- name: Run black to ensure code format
run: poetry run black --diff --check .
- name: Run `ruff format` to ensure code format
run: poetry run ruff format --check .

- name: Run mypy to check types
run: poetry run mypy .
Expand Down
8 changes: 4 additions & 4 deletions docs/sourcetree.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,13 @@ Run `poetry shell` to activate the virtual environment.
Alternatively, use `poetry run` to run a single command in the venv, e.g. `poetry run pytest`.
### Obligatory checks
We force code formatting via `black`, `ruff`, and type hints via `mypy`.
We force code formatting via `ruff`, and type hints via `mypy`.
Run the following commands in the repository's root (next to `pyproject.toml`):

```bash
poetry run black . # All code is reformatted
poetry run ruff . # Python linter
poetry run mypy . # Ensure there are no typing errors
poetry run ruff format . # All code is reformatted
poetry run ruff check . # Python linter
poetry run mypy . # Ensure there are no typing errors
```

**WARNING**: do not run `mypy` from a directory other than the root of the repository.
Expand Down
127 changes: 21 additions & 106 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 12 additions & 12 deletions pre-commit.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,17 @@ def rustfmt(fix_inplace: bool = False, no_color: bool = False) -> str:
return cmd


def black(fix_inplace: bool) -> str:
cmd = "poetry run black"
if not fix_inplace:
cmd += " --diff --check"
def ruff_check(fix_inplace: bool) -> str:
cmd = "poetry run ruff check"
if fix_inplace:
cmd += " --fix"
return cmd


def ruff(fix_inplace: bool) -> str:
cmd = "poetry run ruff"
if fix_inplace:
cmd += " --fix"
def ruff_format(fix_inplace: bool) -> str:
cmd = "poetry run ruff format"
if not fix_inplace:
cmd += " --diff --check"
return cmd


Expand Down Expand Up @@ -109,16 +109,16 @@ def check(name: str, suffix: str, cmd: str, changed_files: List[str], no_color:
no_color=args.no_color,
)
check(
name="black",
name="ruff check",
suffix=".py",
cmd=black(fix_inplace=args.fix_inplace),
cmd=ruff_check(fix_inplace=args.fix_inplace),
changed_files=files,
no_color=args.no_color,
)
check(
name="ruff",
name="ruff format",
suffix=".py",
cmd=ruff(fix_inplace=args.fix_inplace),
cmd=ruff_format(fix_inplace=args.fix_inplace),
changed_files=files,
no_color=args.no_color,
)
Expand Down
16 changes: 5 additions & 11 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,13 @@ pytest-split = "^0.8.1"
zstandard = "^0.21.0"

[tool.poetry.group.dev.dependencies]
black = "^23.3.0"
mypy = "==1.3.0"
ruff = "^0.0.269"
ruff = "^0.1.11"

[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core.masonry.api"

[tool.black]
line-length = 100
extend-exclude = '''
/(
vendor
)/
'''

[tool.mypy]
exclude = "^vendor/"
check_untyped_defs = true
Expand All @@ -82,11 +73,14 @@ ignore_missing_imports = true
[tool.ruff]
target-version = "py39"
extend-exclude = ["vendor/"]
ignore = ["E501"]
ignore = [
"E501", # Line too long, we don't want to be too strict about it
]
select = [
"E", # pycodestyle
"F", # Pyflakes
"I", # isort
"W", # pycodestyle
"B", # bugbear
]
line-length = 100 # this setting is rather guidance, it won't fail if it can't make the shorter
2 changes: 1 addition & 1 deletion scripts/export_import_between_pageservers.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def subprocess_capture(capture_dir: str, cmd: List[str], **kwargs: Any) -> str:
If those files already exist, we will overwrite them.
Returns basepath for files with captured output.
"""
assert type(cmd) is list
assert isinstance(cmd, list)
base = os.path.basename(cmd[0]) + "_{}".format(global_counter())
basepath = os.path.join(capture_dir, base)
stdout_filename = basepath + ".stdout"
Expand Down
4 changes: 2 additions & 2 deletions scripts/reformat
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ set -euox pipefail
echo 'Reformatting Rust code'
cargo fmt
echo 'Reformatting Python code'
poetry run ruff --fix test_runner scripts
poetry run black test_runner scripts
poetry run ruff check --fix test_runner scripts
poetry run ruff format test_runner scripts
4 changes: 2 additions & 2 deletions test_runner/fixtures/neon_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -1101,8 +1101,8 @@ def raw_cli(
If `local_binpath` is true, then we are invoking a test utility
"""

assert type(arguments) == list
assert type(self.COMMAND) == str
assert isinstance(arguments, list)
assert isinstance(self.COMMAND, str)

if local_binpath:
# Test utility
Expand Down
4 changes: 2 additions & 2 deletions test_runner/fixtures/pageserver/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,9 +365,9 @@ def tenant_size_and_modelinputs(self, tenant_id: TenantId) -> Tuple[int, Dict[st
assert isinstance(res, dict)
assert TenantId(res["id"]) == tenant_id
size = res["size"]
assert type(size) == int
assert isinstance(size, int)
inputs = res["inputs"]
assert type(inputs) is dict
assert isinstance(inputs, dict)
return (size, inputs)

def tenant_size_debug(self, tenant_id: TenantId) -> str:
Expand Down
Loading

1 comment on commit 7de829e

@github-actions
Copy link

@github-actions github-actions bot commented on 7de829e Jan 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2306 tests run: 2216 passed, 0 failed, 90 skipped (full report)


Flaky tests (1)

Postgres 14

  • test_empty_tenant_size: debug

Code coverage (full report)

  • functions: 54.5% (10010 of 18361 functions)
  • lines: 81.6% (57508 of 70479 lines)

The comment gets automatically updated with the latest test results
7de829e at 2024-01-05T16:43:09.430Z :recycle:

Please sign in to comment.