fix: settings run id #32
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes how run_id and run directories are handled by introducing timestamped, unique folder creation with retries, updates tests to cover both save and no-save modes, and bumps the package version.
- Added loop in
SwanLabSharedSettingsto generate timestamp-basedrun_dirand retry on collisions - Expanded unit tests in
test_settings.pyto verify timing behavior and save/no-save branches - Updated
pyproject.tomlversion from 0.2.2 to 0.2.3
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/unit/core/test_settings.py | New tests for timestamp alignment, unique dir creation, and no-save mode |
| swankit/core/settings.py | Implemented timestamped run_dir loop with retry and updated assignment |
| pyproject.toml | Bumped project version from 0.2.2 to 0.2.3 |
| # ----------------------- 检测运行文件夹的创建，确保文件夹在当前进程唯一占有 ---------- | ||
| run_dir = None | ||
| while True: | ||
| run_dir is not None and time.sleep(1) |
There was a problem hiding this comment.
Using and for control flow and side-effects here is subtle. Consider replacing this with an explicit if run_dir is not None: time.sleep(1) to improve readability.
| run_dir is not None and time.sleep(1) | |
| if run_dir is not None: | |
| time.sleep(1) |
|
|
||
| # 测试创建文件夹 | ||
| # 尽量保持代码运行在一秒的开始 | ||
| time.sleep(1 - (time.time() % 1)) # 确保时间戳在整秒 |
There was a problem hiding this comment.
Relying on real time.sleep and datetime.now to align to the start of a second may lead to flaky tests. Consider injecting a deterministic clock or mocking datetime.now for more reliable assertions.
| # 测试创建一个存在的文件夹 | ||
| run_id = "123456" | ||
| run_dir = os.path.join(TEMP_DIR, "run-{}-{}".format(datetime.now().strftime("%Y%m%d_%H%M%S"), run_id)) | ||
| os.mkdir(run_dir) |
There was a problem hiding this comment.
The test creates directories under TEMP_DIR but does not clean them up. This can lead to side-effects across runs—consider using fixtures (e.g., tmp_path) or adding teardown steps to remove created folders.
Description
fix settings run id