Skip to content

Add module settings parser coverage - #96

Merged
kissetfall merged 1 commit into
ergohaven:mainfrom
IgorArkhipov:igor/module-settings-tests-fix
Jul 17, 2026
Merged

Add module settings parser coverage#96
kissetfall merged 1 commit into
ergohaven:mainfrom
IgorArkhipov:igor/module-settings-tests-fix

Conversation

@IgorArkhipov

@IgorArkhipov IgorArkhipov commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

EN

Problem

The module-settings audit still lacked direct parser boundary coverage and a test for duplicate QSID read widths. Current main already covers verified writeback success and failure paths, while open PRs #91 and #94 add controller grouping and split-touchpad JSON fixtures. Duplicating those tests here would make this branch overlap unnecessarily.

The parser also converted JSON QSID values with an unchecked as u16 cast, so values above 65535 could wrap into a supported setting ID.

Fix

  • Reject module-setting QSID values that do not fit in u16 instead of allowing integer wraparound.
  • Extract duplicate-QSID width resolution into a focused helper that always selects the widest required HID read.
  • Add parser tests for integer bounds, select metadata normalization, unsupported fields, blank titles, unknown types, and out-of-range QSIDs.
  • Add a duplicate-QSID test covering mixed one-byte and two-byte fields.
  • Keep grouping and split-device fixture coverage in PRs Stabilize module settings grouping #91 and Discover split touchpad settings #94, and keep existing writeback-path tests on main.

Verification

  • cargo test: 195 passed.
  • cargo test module_setting: 11 passed.
  • cargo clippy --all-targets: passed with existing repository warnings.
  • python3 scripts/check_i18n.py: passed.
  • Scoped rustfmt --check and git diff --check: passed. Repository-wide cargo fmt --all -- --check remains blocked by pre-existing formatting differences in src/keycode.rs.
  • TARGET=aarch64-apple-darwin scripts/build_macos_app.sh: arm64 app, DMG, and ZIP built; architecture and code-signature validation passed.

RU

Проблема

В аудите настроек модулей оставались непокрытыми граничные случаи парсера и выбор ширины чтения для повторяющихся QSID. Текущая ветка main уже содержит тесты успешной и ошибочной проверенной записи, а открытые PR #91 и #94 добавляют JSON-фикстуры для группировки контроллеров и разделенных touchpad-настроек. Дублирование этих тестов создало бы лишнее пересечение между ветками.

Кроме того, парсер преобразовывал QSID из JSON через непроверенный as u16, поэтому значения больше 65535 могли переполниться и превратиться в поддерживаемый идентификатор настройки.

Исправление

  • QSID, не помещающиеся в u16, теперь отклоняются без целочисленного переполнения.
  • Логика выбора ширины для повторяющихся QSID вынесена в отдельную функцию, которая всегда выбирает самое широкое требуемое HID-чтение.
  • Добавлены тесты парсера для границ integer-полей, нормализации select-метаданных, неподдерживаемых полей, пустых заголовков, неизвестных типов и QSID вне диапазона.
  • Добавлен тест повторяющегося QSID с одно- и двухбайтовыми полями.
  • Покрытие группировки и split-устройств остается в PR Stabilize module settings grouping #91 и Discover split touchpad settings #94, а существующие тесты записи остаются в main.

Проверка

  • cargo test: прошли 195 тестов.
  • cargo test module_setting: прошли 11 тестов.
  • cargo clippy --all-targets: прошел с существующими предупреждениями репозитория.
  • python3 scripts/check_i18n.py: прошел.
  • Scoped rustfmt --check и git diff --check: прошли. Общий cargo fmt --all -- --check по-прежнему блокируется существующими отличиями форматирования в src/keycode.rs.
  • TARGET=aarch64-apple-darwin scripts/build_macos_app.sh: собраны arm64-приложение, DMG и ZIP; архитектура и подпись прошли проверку.

@kissetfall
kissetfall merged commit 61b4741 into ergohaven:main Jul 17, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants