Skip to content

Commit d785d92

Browse files
committed
address PR feedback
1 parent f0e8bda commit d785d92

File tree

2 files changed

+61
-30
lines changed

2 files changed

+61
-30
lines changed

.generator/cli.py

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,35 +24,56 @@
2424
GENERATE_REQUEST_FILE = "generate-request.json"
2525

2626

27-
# Helper function that reads a json file path and returns the loaded json content.
2827
def _read_json_file(path):
28+
"""Helper function that reads a json file path and returns the loaded json content.
29+
30+
Args:
31+
path (str): The file path to read.
32+
33+
Returns:
34+
dict: The parsed JSON content.
35+
36+
Raises:
37+
FileNotFoundError: If the file is not found at the specified path.
38+
json.JSONDecodeError: If the file does not contain valid JSON.
39+
IOError: If there is an issue reading the file.
40+
"""
2941
with open(path, "r") as f:
3042
return json.load(f)
3143

3244

33-
def handle_configure(dry_run=False):
34-
# TODO(https://github.com/googleapis/librarian/issues/466): Implement configure command.
45+
def handle_configure():
46+
# TODO(https://github.com/googleapis/librarian/issues/466): Implement configure command and update docstring.
3547
logger.info("'configure' command executed.")
3648

3749

38-
def handle_generate(dry_run=False):
50+
def handle_generate():
51+
"""The main coordinator for the code generation process.
52+
53+
This function orchestrates the generation of a client library by reading a
54+
`librarian/generate-request.json` file, determining the necessary Bazel rule for each API, and
55+
(in future steps) executing the build.
56+
57+
Raises:
58+
ValueError: If the `generate-request.json` file is not found or read.
59+
"""
3960

4061
# Read a generate-request.json file
41-
if not dry_run:
42-
try:
43-
request_data = _read_json_file(f"{LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}")
44-
except Exception as e:
45-
logger.error(f"failed to read {LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}: {e}")
46-
sys.exit(1)
62+
try:
63+
request_data = _read_json_file(f"{LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}")
64+
except Exception as e:
65+
raise ValueError(
66+
f"failed to read {LIBRARIAN_DIR}/{GENERATE_REQUEST_FILE}"
67+
) from e
4768

48-
logger.info(json.dumps(request_data, indent=2))
69+
logger.info(json.dumps(request_data, indent=2))
4970

50-
# TODO(https://github.com/googleapis/librarian/issues/448): Implement generate command.
71+
# TODO(https://github.com/googleapis/librarian/issues/448): Implement generate command and update docstring.
5172
logger.info("'generate' command executed.")
5273

5374

54-
def handle_build(dry_run=False):
55-
# TODO(https://github.com/googleapis/librarian/issues/450): Implement build command.
75+
def handle_build():
76+
# TODO(https://github.com/googleapis/librarian/issues/450): Implement build command and update docstring.
5677
logger.info("'build' command executed.")
5778

5879

@@ -62,11 +83,6 @@ def handle_build(dry_run=False):
6283
dest="command", required=True, help="Available commands"
6384
)
6485

65-
# This flag is needed for testing.
66-
parser.add_argument(
67-
"--dry-run", action="store_true", help="Perform a dry run for testing purposes."
68-
)
69-
7086
# Define commands
7187
handler_map = {
7288
"configure": handle_configure,
@@ -87,4 +103,4 @@ def handle_build(dry_run=False):
87103
sys.exit(1)
88104

89105
args = parser.parse_args()
90-
args.func(dry_run=args.dry_run)
106+
args.func()

.generator/test_cli.py

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,14 +49,15 @@ def mock_generate_request_file(tmp_path, monkeypatch):
4949
return request_file
5050

5151

52-
def test_handle_configure_dry_run():
53-
# This is a simple test to ensure that the dry run command succeeds.
54-
handle_configure(dry_run=True)
52+
def test_handle_configure_success(caplog, mock_generate_request_file):
53+
"""
54+
Tests the successful execution path of handle_configure.
55+
"""
56+
caplog.set_level(logging.INFO)
5557

58+
handle_configure()
5659

57-
def test_handle_generate_dry_run():
58-
# This is a simple test to ensure that the dry run command succeeds.
59-
handle_generate(dry_run=True)
60+
assert "'configure' command executed." in caplog.text
6061

6162

6263
def test_handle_generate_success(caplog, mock_generate_request_file):
@@ -65,15 +66,29 @@ def test_handle_generate_success(caplog, mock_generate_request_file):
6566
"""
6667
caplog.set_level(logging.INFO)
6768

68-
handle_generate(dry_run=False)
69+
handle_generate()
6970

7071
assert "google-cloud-language" in caplog.text
7172
assert "'generate' command executed." in caplog.text
7273

7374

74-
def test_handle_build_dry_run():
75-
# This is a simple test to ensure that the dry run command succeeds.
76-
handle_build(dry_run=True)
75+
def test_handle_generate_fail(caplog):
76+
"""
77+
Tests the failed to read `librarian/generate-request.json` file in handle_generates.
78+
"""
79+
with pytest.raises(ValueError):
80+
handle_generate()
81+
82+
83+
def test_handle_build_success(caplog, mock_generate_request_file):
84+
"""
85+
Tests the successful execution path of handle_build.
86+
"""
87+
caplog.set_level(logging.INFO)
88+
89+
handle_build()
90+
91+
assert "'build' command executed." in caplog.text
7792

7893

7994
def test_read_valid_json(mocker):

0 commit comments

Comments
 (0)