Skip to content

Commit 3bc476c

Browse files
tonytony
committed
test: Add comprehensive tests and code quality improvements
why: Ensure all new features have proper test coverage and code meets quality standards what: - Add comprehensive tests for add command with pytest fixtures - Add comprehensive tests for add_from_fs filesystem scanner - Add comprehensive tests for fmt command functionality - Add comprehensive tests for logging utilities - Fix conflicting logging fixtures between test files - Include stderr in CLI test output capture - Apply ruff linting and formatting fixes throughout - Add missing imports and fix type annotations - Improve docstring style consistency refs: Part of scanner-and-add feature implementation
1 parent dad4fd1 commit 3bc476c

8 files changed

Lines changed: 2636 additions & 66 deletions

File tree

‎src/vcspull/cli/add.py‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import logging
66
import pathlib
7+
import traceback
78
import typing as t
89

910
import yaml
@@ -105,8 +106,6 @@ def add_repo(
105106
except Exception:
106107
log.exception("Error loading YAML from %s. Aborting.", config_file_path)
107108
if log.isEnabledFor(logging.DEBUG):
108-
import traceback
109-
110109
traceback.print_exc()
111110
return
112111
else:
@@ -157,7 +156,8 @@ def add_repo(
157156
current_url = str(existing_config)
158157

159158
log.warning(
160-
"Repository '%s' already exists under '%s'. Current URL: %s. To update, remove and re-add, or edit the YAML file manually.",
159+
"Repository '%s' already exists under '%s'. Current URL: %s. "
160+
"To update, remove and re-add, or edit the YAML file manually.",
161161
name,
162162
base_dir_key,
163163
current_url,
@@ -171,16 +171,24 @@ def add_repo(
171171
try:
172172
save_config_yaml(config_file_path, raw_config)
173173
log.info(
174-
f"{Fore.GREEN}✓{Style.RESET_ALL} Successfully added "
175-
f"{Fore.CYAN}'{name}'{Style.RESET_ALL} "
176-
f"({Fore.YELLOW}{url}{Style.RESET_ALL}) to "
177-
f"{Fore.BLUE}{config_file_path}{Style.RESET_ALL} under "
178-
f"'{Fore.MAGENTA}{base_dir_key}{Style.RESET_ALL}'.",
174+
"%s✓%s Successfully added %s'%s'%s (%s%s%s) to %s%s%s under '%s%s%s'.",
175+
Fore.GREEN,
176+
Style.RESET_ALL,
177+
Fore.CYAN,
178+
name,
179+
Style.RESET_ALL,
180+
Fore.YELLOW,
181+
url,
182+
Style.RESET_ALL,
183+
Fore.BLUE,
184+
config_file_path,
185+
Style.RESET_ALL,
186+
Fore.MAGENTA,
187+
base_dir_key,
188+
Style.RESET_ALL,
179189
)
180190
except Exception:
181191
log.exception("Error saving config to %s", config_file_path)
182192
if log.isEnabledFor(logging.DEBUG):
183-
import traceback
184-
185193
traceback.print_exc()
186194
return

‎src/vcspull/cli/add_from_fs.py‎

Lines changed: 94 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import os
77
import pathlib
88
import subprocess
9+
import traceback
910
import typing as t
1011

1112
import yaml
@@ -114,9 +115,13 @@ def add_from_filesystem(
114115
if not home_configs:
115116
config_file_path = pathlib.Path.cwd() / ".vcspull.yaml"
116117
log.info(
117-
f"{Fore.CYAN}i{Style.RESET_ALL} No config specified and no default "
118-
f"home config, will use/create "
119-
f"{Fore.BLUE}{config_file_path}{Style.RESET_ALL}",
118+
"%si%s No config specified and no default "
119+
"home config, will use/create %s%s%s",
120+
Fore.CYAN,
121+
Style.RESET_ALL,
122+
Fore.BLUE,
123+
config_file_path,
124+
Style.RESET_ALL,
120125
)
121126
elif len(home_configs) > 1:
122127
log.error(
@@ -140,15 +145,16 @@ def add_from_filesystem(
140145
except Exception:
141146
log.exception("Error loading YAML from %s. Aborting.", config_file_path)
142147
if log.isEnabledFor(logging.DEBUG):
143-
import traceback
144-
145148
traceback.print_exc()
146149
return
147150
else:
148151
log.info(
149-
f"{Fore.CYAN}i{Style.RESET_ALL} Config file "
150-
f"{Fore.BLUE}{config_file_path}{Style.RESET_ALL} "
151-
f"not found. A new one will be created.",
152+
"%si%s Config file %s%s%s not found. A new one will be created.",
153+
Fore.CYAN,
154+
Style.RESET_ALL,
155+
Fore.BLUE,
156+
config_file_path,
157+
Style.RESET_ALL,
152158
)
153159

154160
found_repos: list[
@@ -164,7 +170,8 @@ def add_from_filesystem(
164170

165171
if not repo_url:
166172
log.warning(
167-
"Could not determine remote URL for git repository at %s. Skipping.",
173+
"Could not determine remote URL for git repository "
174+
"at %s. Skipping.",
168175
repo_path,
169176
)
170177
continue
@@ -197,7 +204,8 @@ def add_from_filesystem(
197204

198205
if not repo_url:
199206
log.warning(
200-
"Could not determine remote URL for git repository at %s. Skipping.",
207+
"Could not determine remote URL for git repository "
208+
"at %s. Skipping.",
201209
item,
202210
)
203211
continue
@@ -223,8 +231,12 @@ def add_from_filesystem(
223231

224232
if not found_repos:
225233
log.info(
226-
f"{Fore.YELLOW}!{Style.RESET_ALL} No git repositories found in "
227-
f"{Fore.BLUE}{scan_dir}{Style.RESET_ALL}. Nothing to add.",
234+
"%s!%s No git repositories found in %s%s%s. Nothing to add.",
235+
Fore.YELLOW,
236+
Style.RESET_ALL,
237+
Fore.BLUE,
238+
scan_dir,
239+
Style.RESET_ALL,
228240
)
229241
return
230242

@@ -242,52 +254,82 @@ def add_from_filesystem(
242254
# Show summary only when there are many existing repos
243255
if len(existing_repos) > 5:
244256
log.info(
245-
f"{Fore.YELLOW}!{Style.RESET_ALL} Found "
246-
f"{Fore.CYAN}{len(existing_repos)}{Style.RESET_ALL} "
247-
f"existing repositories already in configuration.",
257+
"%s!%s Found %s%d%s existing repositories already in configuration.",
258+
Fore.YELLOW,
259+
Style.RESET_ALL,
260+
Fore.CYAN,
261+
len(existing_repos),
262+
Style.RESET_ALL,
248263
)
249264
else:
250265
# Show details only for small numbers
251266
log.info(
252-
f"{Fore.YELLOW}!{Style.RESET_ALL} Found "
253-
f"{Fore.CYAN}{len(existing_repos)}{Style.RESET_ALL} "
254-
f"existing repositories in configuration:",
267+
"%s!%s Found %s%d%s existing repositories in configuration:",
268+
Fore.YELLOW,
269+
Style.RESET_ALL,
270+
Fore.CYAN,
271+
len(existing_repos),
272+
Style.RESET_ALL,
255273
)
256274
for name, url, key in existing_repos:
257275
log.info(
258-
f" {Fore.BLUE}•{Style.RESET_ALL} "
259-
f"{Fore.CYAN}{name}{Style.RESET_ALL} "
260-
f"({Fore.YELLOW}{url}{Style.RESET_ALL}) at "
261-
f"{Fore.MAGENTA}{key}{name}{Style.RESET_ALL} "
262-
f"in {Fore.BLUE}{config_file_path}{Style.RESET_ALL}",
276+
" %s•%s %s%s%s (%s%s%s) at %s%s%s%s in %s%s%s",
277+
Fore.BLUE,
278+
Style.RESET_ALL,
279+
Fore.CYAN,
280+
name,
281+
Style.RESET_ALL,
282+
Fore.YELLOW,
283+
url,
284+
Style.RESET_ALL,
285+
Fore.MAGENTA,
286+
key,
287+
name,
288+
Style.RESET_ALL,
289+
Fore.BLUE,
290+
config_file_path,
291+
Style.RESET_ALL,
263292
)
264293

265294
if not repos_to_add:
266295
if existing_repos:
267296
log.info(
268-
f"{Fore.GREEN}✓{Style.RESET_ALL} All found repositories already exist "
269-
f"in the configuration. {Fore.GREEN}Nothing to do.{Style.RESET_ALL}",
297+
"%s✓%s All found repositories already exist in the configuration. "
298+
"%sNothing to do.%s",
299+
Fore.GREEN,
300+
Style.RESET_ALL,
301+
Fore.GREEN,
302+
Style.RESET_ALL,
270303
)
271304
return
272305

273306
# Show what will be added
274307
log.info(
275-
f"\n{Fore.GREEN}Found {len(repos_to_add)} new "
276-
f"{'repository' if len(repos_to_add) == 1 else 'repositories'} "
277-
f"to add:{Style.RESET_ALL}",
308+
"\n%sFound %d new %s to add:%s",
309+
Fore.GREEN,
310+
len(repos_to_add),
311+
"repository" if len(repos_to_add) == 1 else "repositories",
312+
Style.RESET_ALL,
278313
)
279314
for repo_name, repo_url, _determined_base_key in repos_to_add:
280315
log.info(
281-
f" {Fore.GREEN}+{Style.RESET_ALL} {Fore.CYAN}{repo_name}{Style.RESET_ALL} "
282-
f"({Fore.YELLOW}{repo_url}{Style.RESET_ALL})",
316+
" %s+%s %s%s%s (%s%s%s)",
317+
Fore.GREEN,
318+
Style.RESET_ALL,
319+
Fore.CYAN,
320+
repo_name,
321+
Style.RESET_ALL,
322+
Fore.YELLOW,
323+
repo_url,
324+
Style.RESET_ALL,
283325
)
284326

285327
if not yes:
286328
confirm = input(
287329
f"\n{Fore.CYAN}Add these repositories? [y/N]: {Style.RESET_ALL}",
288330
).lower()
289331
if confirm not in {"y", "yes"}:
290-
log.info(f"{Fore.RED}✗{Style.RESET_ALL} Aborted by user.")
332+
log.info("%s✗%s Aborted by user.", Fore.RED, Style.RESET_ALL)
291333
return
292334

293335
changes_made = False
@@ -305,28 +347,40 @@ def add_from_filesystem(
305347
if repo_name not in raw_config[determined_base_key]:
306348
raw_config[determined_base_key][repo_name] = {"repo": repo_url}
307349
log.info(
308-
f"{Fore.GREEN}+{Style.RESET_ALL} Adding "
309-
f"{Fore.CYAN}'{repo_name}'{Style.RESET_ALL} "
310-
f"({Fore.YELLOW}{repo_url}{Style.RESET_ALL}) under "
311-
f"'{Fore.MAGENTA}{determined_base_key}{Style.RESET_ALL}'.",
350+
"%s+%s Adding %s'%s'%s (%s%s%s) under '%s%s%s'.",
351+
Fore.GREEN,
352+
Style.RESET_ALL,
353+
Fore.CYAN,
354+
repo_name,
355+
Style.RESET_ALL,
356+
Fore.YELLOW,
357+
repo_url,
358+
Style.RESET_ALL,
359+
Fore.MAGENTA,
360+
determined_base_key,
361+
Style.RESET_ALL,
312362
)
313363
changes_made = True
314364

315365
if changes_made:
316366
try:
317367
save_config_yaml(config_file_path, raw_config)
318368
log.info(
319-
f"{Fore.GREEN}✓{Style.RESET_ALL} Successfully updated "
320-
f"{Fore.BLUE}{config_file_path}{Style.RESET_ALL}.",
369+
"%s✓%s Successfully updated %s%s%s.",
370+
Fore.GREEN,
371+
Style.RESET_ALL,
372+
Fore.BLUE,
373+
config_file_path,
374+
Style.RESET_ALL,
321375
)
322376
except Exception:
323377
log.exception("Error saving config to %s", config_file_path)
324378
if log.isEnabledFor(logging.DEBUG):
325-
import traceback
326-
327379
traceback.print_exc()
328380
return
329381
else:
330382
log.info(
331-
f"{Fore.GREEN}✓{Style.RESET_ALL} No changes made to the configuration.",
383+
"%s✓%s No changes made to the configuration.",
384+
Fore.GREEN,
385+
Style.RESET_ALL,
332386
)

‎src/vcspull/cli/fmt.py‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,16 @@ def normalize_repo_config(repo_data: t.Any) -> dict[str, t.Any]:
5656
if isinstance(repo_data, str):
5757
# Convert compact format to verbose format
5858
return {"repo": repo_data}
59-
elif isinstance(repo_data, dict):
59+
if isinstance(repo_data, dict):
6060
# If it has 'url' key but not 'repo', convert to use 'repo'
6161
if "url" in repo_data and "repo" not in repo_data:
6262
normalized = repo_data.copy()
6363
normalized["repo"] = normalized.pop("url")
6464
return normalized
6565
# Already in correct format or has other fields
6666
return repo_data
67-
else:
68-
# Return as-is for other types
69-
return t.cast(dict[str, t.Any], repo_data)
67+
# Return as-is for other types
68+
return t.cast("dict[str, t.Any]", repo_data)
7069

7170

7271
def format_config(config_data: dict[str, t.Any]) -> tuple[dict[str, t.Any], int]:
@@ -202,7 +201,7 @@ def format_single_config(
202201

203202
for directory, repos in raw_config.items():
204203
if isinstance(repos, dict):
205-
for repo_name, repo_data in repos.items():
204+
for repo_data in repos.values():
206205
if isinstance(repo_data, str):
207206
compact_to_verbose += 1
208207
elif isinstance(repo_data, dict):
@@ -273,7 +272,7 @@ def format_single_config(
273272
Fore.CYAN,
274273
Style.RESET_ALL,
275274
)
276-
275+
277276
return True
278277

279278

@@ -296,25 +295,25 @@ def format_config_file(
296295
if format_all:
297296
# Format all discovered config files
298297
config_files = find_config_files(include_home=True)
299-
298+
300299
# Also check for local .vcspull.yaml
301300
local_yaml = pathlib.Path.cwd() / ".vcspull.yaml"
302301
if local_yaml.exists() and local_yaml not in config_files:
303302
config_files.append(local_yaml)
304-
303+
305304
# Also check for local .vcspull.json
306305
local_json = pathlib.Path.cwd() / ".vcspull.json"
307306
if local_json.exists() and local_json not in config_files:
308307
config_files.append(local_json)
309-
308+
310309
if not config_files:
311310
log.error(
312311
"%s✗%s No configuration files found.",
313312
Fore.RED,
314313
Style.RESET_ALL,
315314
)
316315
return
317-
316+
318317
log.info(
319318
"%si%s Found %s%d%s configuration %s to format:",
320319
Fore.CYAN,
@@ -324,7 +323,7 @@ def format_config_file(
324323
Style.RESET_ALL,
325324
"file" if len(config_files) == 1 else "files",
326325
)
327-
326+
328327
for config_file in config_files:
329328
log.info(
330329
" %s•%s %s%s%s",
@@ -334,14 +333,14 @@ def format_config_file(
334333
config_file,
335334
Style.RESET_ALL,
336335
)
337-
336+
338337
log.info("") # Empty line for readability
339-
338+
340339
success_count = 0
341340
for config_file in config_files:
342341
if format_single_config(config_file, write):
343342
success_count += 1
344-
343+
345344
# Summary
346345
if success_count == len(config_files):
347346
log.info(
@@ -385,5 +384,5 @@ def format_config_file(
385384
return
386385
else:
387386
config_file_path = home_configs[0]
388-
387+
389388
format_single_config(config_file_path, write)

0 commit comments

Comments
 (0)