-
Notifications
You must be signed in to change notification settings - Fork 2
Add Tests #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Tests #29
Conversation
- Add pytest and pytest-cov to requirements - Add pytest configuration (pytest.ini) - Add shared test fixtures (conftest.py) - Add tests README documentation - Update .gitignore for test artifacts
- Add Shah algorithm tests with error handling - Add parametrized tests for all algorithms
📝 WalkthroughWalkthroughДобавлена комплексная инфраструктура тестирования проекта: конфигурация GitHub Actions для CI, pytest с покрытием кода, набор модулей тестирования (алгоритмы, утилиты, функции, концов-в-конец), fixture-ы, скрипты для генерации и измерения порогов тестовых данных, а также конфигурационные файлы (.gitignore, pytest.ini, requirements.txt). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (9)
tests/README.md (2)
55-62: Добавьте спецификатор языка для блоков кода.Блоки кода с математическими формулами не имеют спецификатора языка. Рекомендуется добавить
textилиmathдля улучшения совместимости с линтерами.🔎 Предлагаемое исправление
-``` +```text B = Y⁻¹ @ A @ XIn the test data, Y is typically identity (for simplicity), so:
-+text
B = A @ X
80-85: Добавьте спецификатор языка для блока кода.Блок с примером данных не имеет спецификатора языка.
🔎 Предлагаемое исправление
-``` +```text Pose 0: Frame A: [0.00, 0.00, 0.00] mm Frame B: [49.92, 100.02, 24.93] mm (Same physical location, different coordinate systems)</details> </blockquote></details> <details> <summary>tests/generate_test_data.py (2)</summary><blockquote> `29-29`: **Рассмотрите использование `np.random.default_rng()` вместо глобального `np.random.seed()`.** Использование `np.random.seed()` изменяет глобальное состояние генератора случайных чисел, что может привести к неожиданному поведению при параллельном запуске тестов или повторном использовании функции. <details> <summary>🔎 Предлагаемое исправление</summary> ```diff -def add_noise_to_poses(poses, pos_noise_range=5.0, rot_noise_range=0.5, seed=42): +def add_noise_to_poses(poses, pos_noise_range=5.0, rot_noise_range=0.5, seed=42): """ Add noise to a list of poses. Args: poses: List of 4x4 transformation matrices pos_noise_range: Maximum position noise in mm (±range) rot_noise_range: Maximum rotation noise in degrees (±range) seed: Random seed for reproducibility Returns: List of noisy poses """ - np.random.seed(seed) # Fixed seed for reproducibility + rng = np.random.default_rng(seed) # Fixed seed for reproducibility noisy_poses = [] for T in poses: # Extract position and rotation R = T[:3, :3] t = T[:3, 3] # Add position noise (±pos_noise_range mm) - pos_noise = np.random.uniform(-pos_noise_range, pos_noise_range, 3) + pos_noise = rng.uniform(-pos_noise_range, pos_noise_range, 3) t_noisy = t + pos_noise # Add rotation noise (±rot_noise_range degrees) - rot_noise_deg = np.random.uniform(-rot_noise_range, rot_noise_range, 3) + rot_noise_deg = rng.uniform(-rot_noise_range, rot_noise_range, 3) rot_noise_rad = np.deg2rad(rot_noise_deg)
93-139: Рассмотрите вынесение общего кода в вспомогательную функцию.Функции генерации траекторий (
generate_known_transformation_files,generate_circular_trajectory_files, и др.) содержат повторяющийся паттерн: создание директории, генерация B-поз через трансформацию, добавление шума, запись файлов. Это можно абстрагировать для уменьшения дублирования.tests/test_algorithms.py (1)
154-173: Неиспользуемый параметр фикстурыsimple_poses.Методы
test_shah_returns_transformsиtest_shah_rotation_propertiesобъявляют параметрsimple_poses, но не используют его, создавая собственныйvaried_poses. Статический анализатор корректно указывает на это (ARG002).🔎 Предлагаемое исправление
Создайте отдельную фикстуру для Shah или уберите неиспользуемый параметр:
- def test_shah_returns_transforms(self, simple_poses): + def test_shah_returns_transforms(self): """Test that shah returns two transformation matrices""" # Shah algorithm may fail with identical poses, so use more varied poses varied_poses = []- def test_shah_rotation_properties(self, simple_poses): + def test_shah_rotation_properties(self): """Test that returned rotations have correct properties""" # Shah algorithm may fail with identical poses, so use more varied poses varied_poses = []tests/test_end_to_end.py (2)
168-168: Лишний f-prefix в строке.Статический анализатор корректно указывает, что f-строка не содержит плейсхолдеров.
🔎 Предлагаемое исправление
- pytest.skip(f"Test data files not found. Run tests/generate_test_data.py to generate them.") + pytest.skip("Test data files not found. Run tests/generate_test_data.py to generate them.")Аналогичные исправления нужны в строках 183, 196, 209 и 222.
29-145: Рассмотрите загрузку пороговых значений из JSON-файла.
REGRESSION_THRESHOLDSдублирует данные изtests/threshold_measurements.json. Это нарушает принцип DRY и может привести к рассинхронизации при обновлении порогов.🔎 Предлагаемое решение
import json # Load thresholds from the measurements file THRESHOLDS_FILE = Path(__file__).parent / "threshold_measurements.json" if THRESHOLDS_FILE.exists(): with open(THRESHOLDS_FILE) as f: _data = json.load(f) REGRESSION_THRESHOLDS = _data.get("thresholds", {}) else: REGRESSION_THRESHOLDS = {}Это обеспечит единый источник истины для пороговых значений.
tests/test_functions_call.py (1)
157-167: Неиспользуемая переменнаяr_rows.🔎 Предлагаемое исправление
- t_rows, r_rows = get_error_data(["tsai-lenz", "invalid"], sample_csv_file, sample_csv_file) + t_rows, _ = get_error_data(["tsai-lenz", "invalid"], sample_csv_file, sample_csv_file)tests/measure_thresholds.py (1)
52-56: Рассмотрите более специфичную обработку исключений.Перехват
Exceptionслишком широкий и может скрыть неожиданные ошибки. Для утилитарного скрипта это допустимо, но рекомендуется логировать тип исключения для отладки.🔎 Предлагаемое улучшение
try: As, Bs = load_inputs(str(file_a), str(file_b)) - except Exception as e: - print(f"Error loading files: {e}") + except (FileNotFoundError, ValueError, pd.errors.ParserError) as e: + print(f"Error loading files ({type(e).__name__}): {e}") continueАналогичное изменение рекомендуется для строки 84.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/thresholds_table.csvis excluded by!**/*.csv
📒 Files selected for processing (16)
.github/workflows/ci.yml.gitignorepytest.inirequirements.txttests/README.mdtests/__init__.pytests/conftest.pytests/data/.gitkeeptests/generate_test_data.pytests/measure_thresholds.pytests/test_algorithms.pytests/test_end_to_end.pytests/test_functions_call.pytests/test_noise.pytests/test_utils.pytests/threshold_measurements.json
🧰 Additional context used
🧬 Code graph analysis (4)
tests/test_end_to_end.py (1)
src/functions_call.py (3)
load_inputs(32-38)get_error_data(61-85)run_method(14-29)
tests/conftest.py (1)
src/utils.py (3)
compose(12-16)euler_ZYX_to_R(18-35)invert_T(4-10)
tests/measure_thresholds.py (3)
src/functions_call.py (2)
load_inputs(32-38)run_method(14-29)tests/test_end_to_end.py (1)
noise_level(149-151)src/utils.py (1)
stats(94-101)
tests/generate_test_data.py (1)
src/utils.py (3)
compose(12-16)euler_ZYX_to_R(18-35)invert_T(4-10)
🪛 LanguageTool
tests/README.md
[style] ~49-~49: This phrase is redundant. Consider writing “moment” or “time”.
Context: ...sent the same physical pose at the same moment in time, but measured from different reference ...
(MOMENT_IN_TIME)
🪛 markdownlint-cli2 (0.18.1)
tests/README.md
55-55: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
60-60: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
80-80: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.10)
tests/test_end_to_end.py
168-168: f-string without any placeholders
Remove extraneous f prefix
(F541)
183-183: f-string without any placeholders
Remove extraneous f prefix
(F541)
196-196: f-string without any placeholders
Remove extraneous f prefix
(F541)
209-209: f-string without any placeholders
Remove extraneous f prefix
(F541)
222-222: f-string without any placeholders
Remove extraneous f prefix
(F541)
232-232: Unpacked variable noise_level is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
317-317: Unpacked variable noise_level is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
tests/test_functions_call.py
38-38: Redefinition of unused compose from line 14
Remove definition: compose
(F811)
38-38: Redefinition of unused euler_ZYX_to_R from line 14
Remove definition: euler_ZYX_to_R
(F811)
160-160: Unpacked variable r_rows is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
tests/test_algorithms.py
154-154: Unused method argument: simple_poses
(ARG002)
175-175: Unused method argument: simple_poses
(ARG002)
tests/measure_thresholds.py
54-54: Do not catch blind exception: Exception
(BLE001)
84-84: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (16)
tests/__init__.py (1)
1-3: LGTM!Простой файл-маркер пакета для тестовой директории. Комментарий достаточен для идентификации назначения модуля.
.gitignore (2)
15-21: LGTM!Правила игнорирования для артефактов тестирования корректны и охватывают все типичные файлы: кэш pytest, отчёты покрытия и артефакты hypothesis.
23-27: LGTM!Игнорирование файлов IDE и редакторов настроено правильно.
tests/README.md (1)
1-150: LGTM!Отличная документация для тестового набора. README содержит полезную информацию о структуре тестов, генерации данных, математических основах калибровки и инструкции по запуску. Это значительно упрощает понимание и поддержку тестовой инфраструктуры.
tests/generate_test_data.py (2)
70-82: LGTM!Алгоритм извлечения углов Эйлера из матрицы вращения корректен для конвенции ZYX. Обработка сингулярного случая (gimbal lock) при
sy < 1e-6реализована правильно.
343-393: LGTM!CLI-интерфейс хорошо структурирован с понятными аргументами для генерации тестовых данных. Поддержка различных уровней шума и отдельных тест-кейсов обеспечивает гибкость использования.
tests/data/.gitkeep (1)
1-4: LGTM!Файл-заглушка корректно документирует назначение директории и ссылается на скрипт генерации данных.
.github/workflows/ci.yml (3)
36-42: Убедитесь, что настроен секретCODECOV_TOKEN.Начиная с версии 4.x,
codecov/codecov-actionтребует токен для загрузки покрытия. Для корректной работы добавьте секретCODECOV_TOKENв настройки репозитория.🔎 Предлагаемое исправление
- name: Upload coverage to Codecov uses: codecov/codecov-action@v4 with: file: ./coverage.xml flags: unittests name: codecov-umbrella fail_ci_if_error: false + token: ${{ secrets.CODECOV_TOKEN }}
14-16: LGTM!Матрица версий Python (3.9-3.12) обеспечивает хорошее покрытие актуальных версий. Кэширование pip ускорит повторные запуски.
32-34: LGTM!Команда pytest корректно настроена для сбора покрытия с выводом в XML и терминал.
pytest.ini (1)
1-9: LGTM!Конфигурация pytest чистая и следует общепринятым соглашениям. Маркер
slowпозволяет гибко исключать длительные тесты при локальной разработке с помощью-m "not slow".tests/threshold_measurements.json (1)
1-1586: Файл данных с пороговыми значениями выглядит корректно.Структура JSON соответствует ожидаемому формату. Обратите внимание, что некоторые алгоритмы показывают очень высокие ошибки на определённых траекториях (например,
li-wang-wuнаcircular_trajectoryимеет среднюю ошибку трансляции ~304 мм), что может указывать на численные особенности этих алгоритмов при определённых входных данных.tests/test_noise.py (1)
1-239: Хорошо структурированный тестовый модуль для функций генерации шума.Использование условных импортов с декоратором
@pytest.mark.skipif— правильный подход для опциональных модулей. Фикстуры корректно очищают временные файлы. Тесты покрывают основные сценарии: создание файлов, формат вывода, поведение с разными seed-значениями.tests/test_utils.py (1)
1-188: Качественный набор тестов для утилитарных функций.Тесты корректно проверяют математические инварианты (ортогональность, детерминант = 1) и используют фикстуры из conftest.py. Покрытие основных функций выглядит достаточным.
tests/conftest.py (1)
1-93: Хорошо организованные фикстуры для тестового набора.Фикстуры предоставляют детерминированные тестовые данные и корректно очищают временные файлы. Структура соответствует лучшим практикам pytest.
tests/measure_thresholds.py (1)
159-180: Скрипт для измерения порогов выглядит корректно.Логика вычисления порогов (1.2x от среднего или +1.0/+0.1 минимум) обеспечивает разумный запас для регрессионных тестов. Результаты сохраняются в JSON для последующего использования в end-to-end тестах.
| pytest==8.3.4 | ||
| pytest-cov==6.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
latest version of pytest 2025
💡 Result:
The latest pytest release is 9.0.2 (uploaded Dec 6, 2025). [1][2]
Install: pip install -U pytest.
Sources: PyPI (pytest 9.0.2 upload details). [1]
Safety DB summary confirming v9.0.2 as latest. [2]
🌐 Web query:
latest version of pytest-cov 2025
💡 Result:
The latest pytest-cov release in 2025 is 7.0.0 (released Sep 9, 2025). [1][2]
Sources:
[1] PyPI project page (release history showing 7.0.0).
[2] PyPI package page / changelog (details for 7.0.0).
Рассмотрите обновление зависимостей до более свежих версий.
Добавление pytest и pytest-cov с фиксированными версиями обеспечивает воспроизводимость тестовой среды. Однако текущие версии устарели: доступны pytest 9.0.2 (выпущен 6 декабря 2025) и pytest-cov 7.0.0 (выпущен 9 сентября 2025). Рекомендуется обновить зависимости до более актуальных версий.
🤖 Prompt for AI Agents
requirements.txt lines 7-8: the pinned pytest and pytest-cov versions are
outdated; update the entries to use the newer releases (pytest==9.0.2 and
pytest-cov==7.0.0) (or adjust pins to your project's preferred versioning
scheme), then run the test suite and CI to verify compatibility and update any
lockfiles or dependency metadata accordingly.
| @pytest.fixture | ||
| def sample_csv_file(): | ||
| """Create a temporary CSV file with pose data""" | ||
| content = """0 100.0 200.0 300.0 45.0 30.0 15.0 | ||
| 1 101.0 201.0 301.0 46.0 31.0 16.0 | ||
| 2 102.0 202.0 302.0 47.0 32.0 17.0 | ||
| 3 103.0 203.0 303.0 48.0 33.0 18.0 | ||
| """ | ||
| with tempfile.NamedTemporaryFile(mode='w', suffix='.txt', delete=False) as f: | ||
| f.write(content) | ||
| temp_path = f.name | ||
|
|
||
| yield temp_path | ||
|
|
||
| if os.path.exists(temp_path): | ||
| os.unlink(temp_path) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_poses(): | ||
| """Generate a list of sample pose transformation matrices""" | ||
| from utils import compose, euler_ZYX_to_R | ||
| import numpy as np | ||
| poses = [] | ||
| for i in range(5): | ||
| angle = i * np.pi / 8 | ||
| R = euler_ZYX_to_R(angle, 0, 0) | ||
| t = np.array([i * 0.5, i * 0.3, i * 0.1]) | ||
| poses.append(compose(R, t)) | ||
| return np.array(poses) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Дублирование фикстур из conftest.py.
Фикстуры sample_csv_file и sample_poses уже определены в tests/conftest.py. Локальные определения создают дублирование кода и могут привести к рассинхронизации.
🔎 Предлагаемое исправление
Удалите локальные фикстуры и используйте общие из conftest.py:
-@pytest.fixture
-def sample_csv_file():
- """Create a temporary CSV file with pose data"""
- content = """0 100.0 200.0 300.0 45.0 30.0 15.0
-1 101.0 201.0 301.0 46.0 31.0 16.0
-2 102.0 202.0 302.0 47.0 32.0 17.0
-3 103.0 203.0 303.0 48.0 33.0 18.0
-"""
- with tempfile.NamedTemporaryFile(mode='w', suffix='.txt', delete=False) as f:
- f.write(content)
- temp_path = f.name
-
- yield temp_path
-
- if os.path.exists(temp_path):
- os.unlink(temp_path)
-
-
-@pytest.fixture
-def sample_poses():
- """Generate a list of sample pose transformation matrices"""
- from utils import compose, euler_ZYX_to_R
- import numpy as np
- poses = []
- for i in range(5):
- angle = i * np.pi / 8
- R = euler_ZYX_to_R(angle, 0, 0)
- t = np.array([i * 0.5, i * 0.3, i * 0.1])
- poses.append(compose(R, t))
- return np.array(poses)Фикстуры из conftest.py автоматически доступны во всех тестовых модулях.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @pytest.fixture | |
| def sample_csv_file(): | |
| """Create a temporary CSV file with pose data""" | |
| content = """0 100.0 200.0 300.0 45.0 30.0 15.0 | |
| 1 101.0 201.0 301.0 46.0 31.0 16.0 | |
| 2 102.0 202.0 302.0 47.0 32.0 17.0 | |
| 3 103.0 203.0 303.0 48.0 33.0 18.0 | |
| """ | |
| with tempfile.NamedTemporaryFile(mode='w', suffix='.txt', delete=False) as f: | |
| f.write(content) | |
| temp_path = f.name | |
| yield temp_path | |
| if os.path.exists(temp_path): | |
| os.unlink(temp_path) | |
| @pytest.fixture | |
| def sample_poses(): | |
| """Generate a list of sample pose transformation matrices""" | |
| from utils import compose, euler_ZYX_to_R | |
| import numpy as np | |
| poses = [] | |
| for i in range(5): | |
| angle = i * np.pi / 8 | |
| R = euler_ZYX_to_R(angle, 0, 0) | |
| t = np.array([i * 0.5, i * 0.3, i * 0.1]) | |
| poses.append(compose(R, t)) | |
| return np.array(poses) |
🧰 Tools
🪛 Ruff (0.14.10)
38-38: Redefinition of unused compose from line 14
Remove definition: compose
(F811)
38-38: Redefinition of unused euler_ZYX_to_R from line 14
Remove definition: euler_ZYX_to_R
(F811)
🤖 Prompt for AI Agents
In tests/test_functions_call.py around lines 17-46, the fixtures sample_csv_file
and sample_poses are duplicated from tests/conftest.py; remove these local
fixture definitions and any unused imports (e.g., tempfile, os, utils, numpy) so
the test module relies on the shared fixtures from conftest.py, leaving any
tests that reference sample_csv_file or sample_poses unchanged.
Summary by CodeRabbit
Примечания к выпуску
Tests
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.