Skip to content

Commit 5427e40

Browse files
gmmcosta15HugoCLSC
andauthored
fix(configfile): fix bugs, optimize parser, add support for normal an inline comments and add unit tests (#198)
# Description - [ ] Feature - [x] Bug fix - [x] Code refactor - [ ] Documentation This pull request introduces: - Fix data-corruption bug in `_parse_file`: separator normalisation was replacing every `:` and `=` in a line instead of just the first one, silently mangling URLs, tokens, and section names with colons - Fix `get()`, `getint()`, `getfloat()`, `getboolean()` default-handling: missing options with no default now raise `NoOptionError` instead of returning `Sentinel.MISSING`; `get()` no longer passes the fallback through the parser - Fix `add_option(value=None)` writing the literal string `"None"` instead of a value-less option line - Convert all logger calls to `%`-style, add `exc_info=True` and file path to `save_configuration` error, add exception chaining (`from e`) throughout - Ignore regular and inline comments in configfile - Enforce singleton pattern in `get_configparser()`: only one `BlocksScreenConfig` and one `ConfigParser` instance exists for the application lifetime, using double-checked locking; add `reset_configparser()` for test isolation - Enforce `:` as the sole stored separator: `_RE_SEP_NORMALIZE` accepts both `=` and `:` from disk and normalises to `: `; all downstream code (`_RE_OPTION`, `_find_option_line_index`, `ConfigParser(delimiters=...)`) restricted to `:` only - Remove unused `check_file_on_path` import; replace with `Path.exists()` on module-level path constants - Add 80 unit tests covering all of the above plus thread safety, view sharing, and factory behaviour # Motivation The `configfile` module had several subtle parsing bugs and inconsistent behaviors that could corrupt configuration values (e.g., URLs, tokens, or section names) and return incorrect results in some edge cases. This PR stabilizes the module by fixing multiple correctness issues, tightening error handling, and improving logging. The singleton refactor ensures all consumers share one parsed state so that `update_option`/`save_configuration` mutations are immediately visible everywhere without a reload. The separator enforcement removes misleading redundancy — `=` is accepted on read but normalised before storage, so no downstream code needs to handle it. To prevent regressions and make future changes safer, a comprehensive unit test suite (80 tests) was added, covering parsing behaviour, comment handling, deduplication, all read methods, view sharing, add/update/save lifecycle, factory and singleton behaviour, RLock reentrancy, and thread safety. # Tests `tests/util/test_configfile_unit.py`: 80 unit tests. Covers parsing, comment handling, deduplication, all read methods, `get_section` view sharing, add/update/save lifecycle, `get_configparser` singleton factory, `reset_configparser`, RLock reentrancy, thread safety, and stale-view-after-reload. --------- Co-authored-by: HugoCLSC <hugo.santos.costa@gmail.com>
1 parent 3c17cb4 commit 5427e40

4 files changed

Lines changed: 1064 additions & 326 deletions

File tree

0 commit comments

Comments
 (0)