Skip to content

Commit 85d91bf

Browse files
authored
[CLI] Migrate format (#5035)
### Changes This PR introduces a new `format` command to the `casp` CLI, which acts as a convenient wrapper for the root `butler.py format` script. It also adds a `casp.utils.path_utils` module that handles finding paths and directories. To enable `casp` to target specific directories for formatting, the `butler.py format` command was updated to accept a `--dir` argument. ### How it works: - `casp format` without arguments will format all changed files in the current branch, preserving the default `butler` behavior. - `casp format <path>` or `casp format --dir <path>` will now run the formatter exclusively on the specified directory. ### Changes in butler Regarding the refactor/improvement in `butler.py`, here are some screenshots comparing running `python butler.py format` in this branch and master. They show that the behavior is the same. <img width="1152" height="48" alt="image" src="https://github.com/user-attachments/assets/84975ad9-dc29-41a8-a199-f18c3fc33cd1" /> <img width="1152" height="49" alt="image" src="https://github.com/user-attachments/assets/a3f77e21-8f84-4304-b95b-479c38bd250c" /> ### Demo prints Here are some screenshots of `casp format` in action: <img width="1932" height="1109" alt="image" src="https://github.com/user-attachments/assets/7144b1b7-b922-4d72-b521-ad7a85082e36" />
1 parent 000dc1f commit 85d91bf

File tree

7 files changed

+294
-9
lines changed

7 files changed

+294
-9
lines changed

butler.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,19 @@ def _setup_args_for_remote(parser):
9898
subparsers.add_parser('reboot', help='Reboot with `sudo reboot`.')
9999

100100

101+
def _add_format_subparser(toplevel_subparsers):
102+
"""Adds a parser for the `format` command."""
103+
parser = toplevel_subparsers.add_parser(
104+
'format', help='Format changed code in current branch.')
105+
parser.add_argument(
106+
'--path',
107+
dest='path',
108+
default=None,
109+
help=
110+
'The file or directory to format. Default is to format changed files in '
111+
'the current branch.')
112+
113+
101114
def _add_integration_tests_subparsers(toplevel_subparsers):
102115
"""Adds a parser for the `integration_tests` command."""
103116
toplevel_subparsers.add_parser(
@@ -295,8 +308,6 @@ def main():
295308
help=('Do not close browser when tests '
296309
'finish. Good for debugging.'))
297310

298-
subparsers.add_parser('format', help='Format changed code in current branch.')
299-
300311
parser_lint = subparsers.add_parser(
301312
'lint', help='Lint changed code in current branch.')
302313
parser_lint.add_argument(
@@ -428,6 +439,7 @@ def main():
428439
default='us-central',
429440
help='Location for App Engine.')
430441

442+
_add_format_subparser(subparsers)
431443
_add_integration_tests_subparsers(subparsers)
432444
_add_weights_subparser(subparsers)
433445
_add_reproduce_subparser(subparsers)

cli/casp/src/casp/commands/format.py

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,43 @@
1111
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
14-
"""Format command."""
14+
"""Run format command."""
1515

16+
import subprocess
17+
import sys
18+
19+
from casp.utils import local_butler
1620
import click
1721

1822

19-
@click.command(name='format', help='Format changed code in current branch.')
20-
def cli():
21-
"""Format changed code in current branch."""
22-
click.echo('To be implemented...')
23+
@click.command(name='format', help='Run format command')
24+
@click.argument('path', required=False, type=click.Path(exists=True))
25+
@click.option(
26+
'--path',
27+
'-p',
28+
'path_option',
29+
help='The file or directory to run the format command in.',
30+
default=None,
31+
type=click.Path(exists=True),
32+
show_default=True)
33+
def cli(path: str, path_option: str) -> None:
34+
"""Run format command"""
35+
target_dir = path_option or path
36+
37+
try:
38+
if target_dir:
39+
command = local_butler.build_command('format', path=target_dir)
40+
else:
41+
command = local_butler.build_command('format')
42+
except FileNotFoundError:
43+
click.echo('butler.py not found in this directory.', err=True)
44+
sys.exit(1)
45+
46+
try:
47+
subprocess.run(command, check=True)
48+
except FileNotFoundError:
49+
click.echo('python not found in PATH.', err=True)
50+
sys.exit(1)
51+
except subprocess.CalledProcessError as e:
52+
click.echo(f'Error running butler.py format: {e}', err=True)
53+
sys.exit(1)
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
"""Tests for the format command.
15+
16+
For running all the tests, use (from the root of the project):
17+
python -m unittest discover -s cli/casp/src/casp/tests -p test_format.py -v
18+
"""
19+
20+
from pathlib import Path
21+
import subprocess
22+
import unittest
23+
from unittest.mock import patch
24+
25+
from casp.commands import format as format_command
26+
from click.testing import CliRunner
27+
28+
29+
class FormatCliTest(unittest.TestCase):
30+
"""Tests for the format command."""
31+
32+
def setUp(self):
33+
self.runner = CliRunner()
34+
self.mock_local_butler = self.enterContext(
35+
patch('casp.commands.format.local_butler', autospec=True))
36+
self.mock_subprocess_run = self.enterContext(
37+
patch('subprocess.run', autospec=True))
38+
39+
def test_format_success_no_dir(self):
40+
"""Tests successful execution of `casp format` without a directory."""
41+
self.mock_local_butler.build_command.return_value = ['cmd']
42+
result = self.runner.invoke(format_command.cli)
43+
self.assertEqual(0, result.exit_code, msg=result.output)
44+
self.mock_local_butler.build_command.assert_called_once_with('format')
45+
self.mock_subprocess_run.assert_called_once_with(['cmd'], check=True)
46+
47+
def test_format_success_with_path_option(self):
48+
"""Tests `casp format --path <some_dir>`."""
49+
self.mock_local_butler.build_command.return_value = ['cmd']
50+
with self.runner.isolated_filesystem():
51+
Path('test_dir').mkdir()
52+
result = self.runner.invoke(format_command.cli, ['--path', 'test_dir'])
53+
self.assertEqual(0, result.exit_code, msg=result.output)
54+
self.mock_local_butler.build_command.assert_called_once_with(
55+
'format', path='test_dir')
56+
self.mock_subprocess_run.assert_called_once_with(['cmd'], check=True)
57+
58+
def test_format_success_with_path(self):
59+
"""Tests `casp format <some_dir>` (positional argument)."""
60+
self.mock_local_butler.build_command.return_value = ['cmd']
61+
with self.runner.isolated_filesystem():
62+
Path('test_dir').mkdir()
63+
result = self.runner.invoke(format_command.cli, ['test_dir'])
64+
self.assertEqual(0, result.exit_code, msg=result.output)
65+
self.mock_local_butler.build_command.assert_called_once_with(
66+
'format', path='test_dir')
67+
self.mock_subprocess_run.assert_called_once_with(['cmd'], check=True)
68+
69+
def test_butler_not_found(self):
70+
"""Tests when `butler.py` is not found."""
71+
self.mock_local_butler.build_command.side_effect = FileNotFoundError
72+
result = self.runner.invoke(format_command.cli)
73+
self.assertNotEqual(0, result.exit_code)
74+
self.assertIn('butler.py not found', result.output)
75+
self.mock_subprocess_run.assert_not_called()
76+
77+
def test_subprocess_run_fails(self):
78+
"""Tests when `subprocess.run` fails."""
79+
self.mock_local_butler.build_command.return_value = ['cmd']
80+
self.mock_subprocess_run.side_effect = subprocess.CalledProcessError(
81+
1, 'cmd')
82+
result = self.runner.invoke(format_command.cli)
83+
self.assertNotEqual(0, result.exit_code)
84+
self.assertIn('Error running butler.py format', result.output)
85+
86+
def test_python_not_found(self):
87+
"""Tests when `python` command is not found."""
88+
self.mock_local_butler.build_command.return_value = ['cmd']
89+
self.mock_subprocess_run.side_effect = FileNotFoundError
90+
result = self.runner.invoke(format_command.cli)
91+
self.assertNotEqual(0, result.exit_code)
92+
self.assertIn('python not found in PATH', result.output)
93+
94+
95+
if __name__ == '__main__':
96+
unittest.main()
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
"""Tests for local_butler utility functions.
15+
16+
For running, use (from the root of the project):
17+
python -m unittest discover -s cli/casp/src/casp/tests
18+
-p test_local_butler.py -v
19+
"""
20+
21+
from pathlib import Path
22+
import unittest
23+
from unittest.mock import patch
24+
25+
from casp.utils import local_butler
26+
27+
28+
class BuildCommandTest(unittest.TestCase):
29+
"""Tests for build_command."""
30+
31+
@patch('casp.utils.path_utils.get_butler_in_dir', autospec=True)
32+
def test_build_command_success_auto_find(self, mock_get_butler):
33+
"""Tests successful command build finding butler automatically."""
34+
mock_get_butler.return_value = Path('/fake/butler.py')
35+
36+
command = local_butler.build_command('format', some_arg='value')
37+
38+
self.assertEqual(
39+
command, ['python', '/fake/butler.py', 'format', '--some-arg=value'])
40+
mock_get_butler.assert_called_once()
41+
42+
def test_build_command_success_explicit_path(self):
43+
"""Tests successful command build with explicit butler path."""
44+
butler_path = Path('/explicit/butler.py')
45+
46+
command = local_butler.build_command(
47+
'format', butler_path=butler_path, verbose='true')
48+
49+
self.assertEqual(
50+
command, ['python', '/explicit/butler.py', 'format', '--verbose=true'])
51+
52+
@patch('casp.utils.path_utils.get_butler_in_dir', autospec=True)
53+
def test_butler_not_found(self, mock_get_butler):
54+
"""Tests validation when butler.py is not found."""
55+
mock_get_butler.return_value = None
56+
57+
with self.assertRaises(FileNotFoundError):
58+
local_butler.build_command('format')
59+
60+
61+
if __name__ == '__main__':
62+
unittest.main()
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
"""Local butler command utilities."""
15+
16+
from pathlib import Path
17+
from typing import Optional
18+
19+
from casp.utils import path_utils
20+
21+
22+
def build_command(subcommand: str,
23+
butler_path: Optional[Path] = None,
24+
**kwargs: str) -> list[str]:
25+
"""Builds a butler command for local execution.
26+
27+
Args:
28+
subcommand: The butler subcommand to execute (e.g., 'format').
29+
butler_path: The path to the butler.py file. If not provided, it will be
30+
searched in the current directory.
31+
**kwargs: A dictionary of command-line arguments to pass to the subcommand.
32+
For example, `testcase_id='123'` becomes
33+
'--testcase-id=123'.
34+
35+
Returns:
36+
A list of strings representing the command to be executed.
37+
38+
Raises:
39+
FileNotFoundError: If butler.py is not found.
40+
"""
41+
if butler_path is None:
42+
butler_path = path_utils.get_butler_in_dir(Path.cwd())
43+
44+
if not butler_path:
45+
raise FileNotFoundError('butler.py not found.')
46+
47+
command = ['python', str(butler_path), subcommand]
48+
for key, value in kwargs.items():
49+
key = key.replace('_', '-')
50+
command.append(f'--{key}={value}')
51+
52+
return command
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
"""Manages the CASP path search and management.
15+
16+
This module provides a set of utilities related
17+
to file and directory path manipulation and discovery.
18+
"""
19+
20+
import os
21+
from pathlib import Path
22+
23+
24+
def get_butler_in_dir(start_path: Path) -> Path | None:
25+
"""Find and returns the butler.py script in the current directory"""
26+
current_path = os.path.abspath(start_path)
27+
butler_path = os.path.join(current_path, 'butler.py')
28+
if os.path.exists(butler_path):
29+
return Path(butler_path)
30+
return None

src/local/butler/format.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@
2828
[f'-p {mod}' for mod in FIRST_PARTY_MODULES]) + ' ')
2929

3030

31-
def execute(_):
31+
def execute(args):
3232
"""Format changed code."""
33-
if os.path.exists('.git/FETCH_HEAD'):
33+
if args.path:
34+
diff_command = f'git ls-files {os.path.abspath(args.path)}'
35+
elif os.path.exists('.git/FETCH_HEAD'):
3436
diff_command = 'git diff --name-only FETCH_HEAD'
3537
else:
3638
# If not, fall back to diffing against HEAD.

0 commit comments

Comments
 (0)