From 05a27c3c893faf82383c1858cecc5492c9ed6076 Mon Sep 17 00:00:00 2001 From: ShaoLOWu <310076244+ShaoLOWu@users.noreply.github.com> Date: Mon, 12 Oct 2026 15:27:12 +0800 Subject: [PATCH] fix(core): stop generate_files from clobbering the required_paths global The directory-creation loop iterated as "for required_paths in required_paths:" while declaring "global required_paths", so after the first call the module-level list was rebound to the last iterated string ('data/labels'). A second call then iterated that string character by character and attempted os.mkdir('d'), os.mkdir('a'), ... which raises FileExistsError and aborts boot. Rename the loop variable and drop the now-unneeded global declaration; add regression tests pinning the module state and the repeat-call behaviour. --- src/langbot/pkg/core/bootutils/files.py | 8 ++-- tests/unit_tests/core/test_bootutils_files.py | 48 +++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 tests/unit_tests/core/test_bootutils_files.py diff --git a/src/langbot/pkg/core/bootutils/files.py b/src/langbot/pkg/core/bootutils/files.py index 067b60c10..660f945bf 100644 --- a/src/langbot/pkg/core/bootutils/files.py +++ b/src/langbot/pkg/core/bootutils/files.py @@ -18,13 +18,11 @@ required_paths = [ async def generate_files() -> list[str]: - global required_files, required_paths - from ...utils import paths as path_utils - for required_paths in required_paths: - if not os.path.exists(required_paths): - os.mkdir(required_paths) + for path in required_paths: + if not os.path.exists(path): + os.mkdir(path) generated_files = [] for file in required_files: diff --git a/tests/unit_tests/core/test_bootutils_files.py b/tests/unit_tests/core/test_bootutils_files.py new file mode 100644 index 000000000..c2a59209f --- /dev/null +++ b/tests/unit_tests/core/test_bootutils_files.py @@ -0,0 +1,48 @@ +"""Regression tests for bootutils.files.generate_files. + +Historically the directory-creation loop iterated as +``for required_paths in required_paths:`` while the function declared +``global required_paths``, which rebound the module-level list to the last +iterated string (``'data/labels'``) after the first call. A second call then +iterated over that string character by character and attempted to create +single-character directories. These tests pin the module-level state and the +repeat-call behaviour. +""" + +from __future__ import annotations + +import asyncio +from importlib import import_module + + +def get_files_module(): + """Lazy import to avoid circular import issues.""" + return import_module('langbot.pkg.core.bootutils.files') + + +def test_required_paths_stays_a_list_after_generate_files(tmp_path, monkeypatch): + files = get_files_module() + monkeypatch.chdir(tmp_path) + original = list(files.required_paths) + + asyncio.run(files.generate_files()) + + assert isinstance(files.required_paths, list) + assert files.required_paths == original + + +def test_generate_files_is_idempotent(tmp_path, monkeypatch): + files = get_files_module() + monkeypatch.chdir(tmp_path) + + first = asyncio.run(files.generate_files()) + second = asyncio.run(files.generate_files()) + + for path in files.required_paths: + assert (tmp_path / path).is_dir() + # The config template is copied on the first run only. + assert first == ['data/config.yaml'] + assert second == [] + assert (tmp_path / 'data' / 'config.yaml').is_file() + # No junk single-character directories left behind by a second run. + assert sorted(p.name for p in tmp_path.iterdir()) == ['data', 'temp']