Fix command generation logic and add testing for it #25
Conversation
WalkthroughThis update introduces new test infrastructure, protobuf definitions, and documentation enhancements. It modifies the logic for removing stops in the autonomy command, updates the project version, and expands documentation to cover connection loss handling. Several new Python files and dependencies are added to support manual testing and MQTT monitoring. Changes
Sequence Diagram(s)sequenceDiagram
participant Tester
participant DockerCompose
participant MQTTBroker
participant MqttMonitoring
participant Vehicle
Tester->>DockerCompose: Start all services
DockerCompose->>MQTTBroker: Start
Tester->>MqttMonitoring: Connect and subscribe to topics
loop Wait for first stop
MqttMonitoring->>Vehicle: Monitor status (wait for "Svatopluka Čecha A")
end
Tester->>Vehicle: Send start command
Tester->>DockerCompose: Kill MQTTBroker
Vehicle--xMQTTBroker: Loses connection
Note right of Vehicle: Vehicle continues to next stop offline
Tester->>DockerCompose: Restart MQTTBroker after delay
MQTTBroker->>Vehicle: Connection restored
Vehicle->>MqttMonitoring: Sends status with finished stops in error message
Tester->>MqttMonitoring: Retrieve and verify status
Estimated code review effort
Overall review effort: 3 (30–60 minutes) Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
tests/libs/testing_utilities.py (1)
62-72: Add error handling for API calls.The function should handle potential API errors to provide better debugging information when tests fail.
def get_status_errors() -> list[fleet_http_client_python.Message]: """Get all status errors from the fleet HTTP API.""" - configuration = fleet_http_client_python.Configuration(host="http://localhost:8080/v2/protocol", - api_key={"AdminAuth": "ProtocolStaticAccessKey"}) - api_client = fleet_http_client_python.ApiClient(configuration) - device_api = fleet_http_client_python.DeviceApi(api_client) - statuses = device_api.list_statuses(company_name='bringauto', - car_name='virtual_vehicle', - since=0) - return [status for status in statuses if status.payload.message_type == 'STATUS_ERROR' and status.device_id.module_id == 1] + try: + configuration = fleet_http_client_python.Configuration(host="http://localhost:8080/v2/protocol", + api_key={"AdminAuth": "ProtocolStaticAccessKey"}) + api_client = fleet_http_client_python.ApiClient(configuration) + device_api = fleet_http_client_python.DeviceApi(api_client) + statuses = device_api.list_statuses(company_name='bringauto', + car_name='virtual_vehicle', + since=0) + return [status for status in statuses if status.payload.message_type == 'STATUS_ERROR' and status.device_id.module_id == 1] + except Exception as e: + print(f"Failed to get status errors from fleet API: {e}") + return []
🧹 Nitpick comments (19)
.gitignore (1)
3-3: Add common virtual-env patterns to cover all tooling.
.venvis a good catch, but local setups sometimes generatevenv/or.virtualenv/as well. Ignoring the superset avoids accidental commits from other developers..vscode .venv +venv +.virtualenv _*/ cmake-build-debug/tests/requirements.txt (1)
6-7: Missing#egg=fragments may break editable installs on older pip.For VCS dependencies you normally append an egg name:
-fleet_management_http_client_python @ git+https://github.com/bringauto/fleet-management-http-client-python.git@v3.1.5 +git+https://github.com/bringauto/fleet-management-http-client-python.git@v3.1.5#egg=fleet_management_http_client_pythonSame for
fleet_http_client_python. Pip ≥ 23 tolerates the current form, but older environments and some internal tooling still expect the#egg=syntax.README.md (1)
59-62: Minor formatting & link clarity.Sentence lacks a period and the link would read better in parentheses.
-## Testing - -All tests are contained within the `tests` folder. [Testing README](./tests/README.md) +## Testing + +All tests are contained in the `tests` folder (see the [Testing README](./tests/README.md)).docs/mission_module.md (1)
22-35: Fix typos and diagram condition to aid reader comprehension.Small spelling issues and an incorrect state variable name (
satus) reduce clarity.- in_stop -->|yes| remove_check{{DRIVE car state in previous satus OR next station in command equal to next station in current status}} + in_stop -->|yes| remove_check{{DRIVE car state in previous status OR next station in command equals next station in current status}}Also consider explaining the “previous status” source (e.g., cached last heartbeat) just above the diagram.
tests/README.md (2)
24-24: Minor grammatical improvement suggested.Consider adding a comma for better readability: "After roughly 2m 30s**,** the car should reach the next stop automatically without mqtt connection."
25-26: Add article for better grammar.Consider adding "The" at the beginning: "The path to the etna docker-compose file needs to be provided as an argument."
tests/test_disconnect_on_stop_arrival.py (3)
9-10: Remove excessive blank lines.Consider reducing the blank lines to follow PEP 8 guidelines (maximum 2 consecutive blank lines).
- +
35-37: Use sys.exit() instead of exit().For better clarity and to avoid potential issues, use
sys.exit()instead ofexit().+import sys + if not args.yaml: print('No YAML file provided. Please use --yaml to specify the path to the Docker Compose YAML file.') - exit(1) + sys.exit(1)
58-58: Clean up commented debug code.Remove the commented print statements or add proper comment formatting if they're meant to stay for debugging purposes.
- #print(statuses) + # print(statuses) # Debug: uncomment to see status messages- #print(statuses) + # print(statuses) # Debug: uncomment to see status errorsAlso applies to: 76-76
tests/libs/testing_utilities.py (3)
1-12: Fix formatting issues.Apply these formatting fixes:
import subprocess -import fleet_http_client_python # type: ignore - - +import fleet_http_client_python # type: ignore PROFILE_ALL = 'all'
14-16: Remove extra whitespace.def get_baseline_docker_command(yaml_file: str) -> list[str]: - return ['docker', 'compose', '--file', yaml_file ] + return ['docker', 'compose', '--file', yaml_file]
46-52: Simplify using theany()builtin.def check_car_state(statuses: list, state: int) -> bool: """Check if any of the statuses has the given state.""" - for status in statuses: - if status.state == state: - return True - return False + return any(status.state == state for status in statuses)tests/libs/sniff_mqtt.py (7)
1-1: Split multiple imports onto separate lines.-import time, threading +import time +import threading
3-5: Consider using absolute imports.The current relative imports assume a specific directory structure. Consider using absolute imports for better clarity:
-import libs.ExternalProtocol_pb2 -import libs.InternalProtocol_pb2 -import libs.MissionModule_pb2 +from tests.libs import ExternalProtocol_pb2 +from tests.libs import InternalProtocol_pb2 +from tests.libs import MissionModule_pb2Then update all usages to remove the
libs.prefix.
45-45: Add missing whitespace around operator.- self._client =Client( + self._client = Client(
84-84: Split multiple statements onto separate lines.- if self._verbose: self._print(f'topic: {message.topic}') + if self._verbose: + self._print(f'topic: {message.topic}')
100-100: Use direct attribute access instead of getattr with constants.- self._handle_status(getattr(message, 'status').deviceStatus) + self._handle_status(message.status.deviceStatus)- self._handle_command(getattr(message, 'command')) + self._handle_command(message.command)Also applies to: 114-114
106-107: Remove unnecessary f-string prefixes and fix formatting.- self._print(f'\tMessage with unhandled type') - if self._verbose: self._print(message) + self._print('\tMessage with unhandled type') + if self._verbose: + self._print(message)- self._print(f'\tMessage with unhandled type') - if self._verbose: self._print(message) + self._print('\tMessage with unhandled type') + if self._verbose: + self._print(message)Also applies to: 120-121
208-208: Remove unnecessary f-string prefix.- topic=f'bringauto/virtual_vehicle/external_server', + topic='bringauto/virtual_vehicle/external_server',
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.gitignore(1 hunks)CMakeLists.txt(1 hunks)README.md(1 hunks)docs/mission_module.md(1 hunks)source/bringauto/modules/mission_module/devices/AutonomyDevice.cpp(1 hunks)tests/README.md(1 hunks)tests/libs/ExternalProtocol_pb2.py(1 hunks)tests/libs/InternalProtocol_pb2.py(1 hunks)tests/libs/MissionModule_pb2.py(1 hunks)tests/libs/sniff_mqtt.py(1 hunks)tests/libs/testing_utilities.py(1 hunks)tests/requirements.txt(1 hunks)tests/test_disconnect_on_stop_arrival.py(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: Mayt34
PR: bringauto/transparent-module#1
File: source/bringauto/modules/devices/testing_device/testing_module_manager.cpp:13-23
Timestamp: 2024-10-09T05:03:50.249Z
Learning: In the `testing_device_generate_command` function for the testing device, the command only depends on `current_command` generated by the device, similar to the io-module for button implementation.
Learnt from: Mayt34
PR: bringauto/transparent-module#1
File: source/bringauto/modules/devices/testing_device/testing_module_manager.cpp:13-23
Timestamp: 2024-10-03T11:16:43.026Z
Learning: In the `testing_device_generate_command` function for the testing device, the command only depends on `current_command` generated by the device, similar to the io-module for button implementation.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/mqtt/test_adapter.py:39-41
Timestamp: 2024-10-17T13:07:59.736Z
Learning: In `tests/mqtt/test_adapter.py` and other test files, prefer using fixed values for test configurations like `TEST_ADDRESS` and `TEST_PORT` instead of environment variables, to avoid requiring users to configure the environment for testing.
CMakeLists.txt (4)
Learnt from: koudis
PR: bringauto/module-gateway#43
File: CMakeLists.txt:11-14
Timestamp: 2025-06-29T18:12:34.666Z
Learning: In the ModuleGateway project's CMakeLists.txt, compile definitions for version strings are intentionally set with escaped quotes using CMDEF_COMPILE_DEFINITIONS to produce compiler flags like `-DMODULE_GATEWAY_VERSION="1.3.3"`, ensuring the macro expands to a proper string literal in C++ code.
Learnt from: koudis
PR: bringauto/packager#28
File: example/docker/fedora41/Dockerfile:20-21
Timestamp: 2025-01-21T11:34:52.410Z
Learning: Version pinning for the cmakelib repository in Dockerfiles is not required as per team preference.
Learnt from: Mayt34
PR: bringauto/transparent-module#1
File: CMakeLists.txt:15-20
Timestamp: 2024-10-03T17:41:56.410Z
Learning: In the `transparent_module` project, all modules should be built by default; `FLEET_PROTOCOL_BUILD_MODULE_GATEWAY` and `FLEET_PROTOCOL_BUILD_EXTERNAL_SERVER` should be set to ON by default.
Learnt from: Mayt34
PR: bringauto/transparent-module#1
File: CMakeLists.txt:15-20
Timestamp: 2024-10-09T05:03:50.249Z
Learning: In the `transparent_module` project, all modules should be built by default; `FLEET_PROTOCOL_BUILD_MODULE_GATEWAY` and `FLEET_PROTOCOL_BUILD_EXTERNAL_SERVER` should be set to ON by default.
docs/mission_module.md (5)
Learnt from: MarioIvancik
PR: bringauto/mission-module#19
File: CMakeLists.txt:13-13
Timestamp: 2024-10-23T09:33:50.060Z
Learning: In the mission module project, due to bugs in the protobuf library, messages are always sent as raw JSON, and protobuf is used solely for data validation.
Learnt from: MarioIvancik
PR: bringauto/virtual-vehicle#11
File: source/bringauto/communication/fleet_protocol/FleetProtocol.cpp:177-177
Timestamp: 2024-10-17T08:35:13.267Z
Learning: In the C++ file `source/bringauto/communication/fleet_protocol/FleetProtocol.cpp`, the method `status.getNextStop()` always returns an object with default-initialized members, so accessing its members without null checks is safe.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server.py:525-677
Timestamp: 2024-10-15T08:23:18.096Z
Learning: In the `CarServer` class, no other message types are expected beyond those currently handled.
Learnt from: MarioIvancik
PR: bringauto/module-gateway#39
File: source/bringauto/external_client/connection/ExternalConnection.cpp:309-313
Timestamp: 2025-01-07T12:43:09.880Z
Learning: In the ModuleGateway project, the `toExternalQueue_` in ModuleHandler is used for sending messages to the external server and needs to be monitored to prevent deadlocks, while `commandQueue_` is used for a different purpose of handling device commands.
Learnt from: MarioIvancik
PR: bringauto/virtual-vehicle#11
File: source/bringauto/communication/fleet_protocol/FleetProtocol.cpp:155-155
Timestamp: 2024-10-17T07:23:23.169Z
Learning: Commands are validated before being sent to the virtual vehicle.
README.md (12)
Learnt from: mikusaq
PR: bringauto/packager#24
File: README.md:0-0
Timestamp: 2024-10-09T05:03:50.249Z
Learning: The TODO section in `README.md` was outdated and has been removed.
Learnt from: mikusaq
PR: bringauto/packager#24
File: README.md:0-0
Timestamp: 2024-09-30T11:48:14.934Z
Learning: The TODO section in `README.md` was outdated and has been removed.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase has comprehensive tenant isolation testing across multiple test files:
1. Security tests in tests/security/test_tenants.py
2. Database tests in tests/database/test_using_tenants.py
3. Car-specific tenant isolation in tests/controllers/car/test_car_controller.py
4. API endpoint tests in tests/controllers/tenant/test_tenant.py
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/models/module.py:0-0
Timestamp: 2025-05-12T07:30:17.701Z
Learning: Files in the server/fleetv2_http_api/models/ directory are auto-generated by OpenAPI Generator and should not be manually modified. Any necessary changes should be made to the OpenAPI specification instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/tenant_controller.py:0-0
Timestamp: 2025-01-31T13:44:43.696Z
Learning: Files under `fleet_management_api/controllers/` are auto-generated from OpenAPI specification and should not be modified directly. Any changes to these files should be made by modifying the OpenAPI specification files.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/stop_controller.py:0-0
Timestamp: 2025-01-31T13:42:20.848Z
Learning: Files under fleet_management_api/controllers/ are auto-generated from OpenAPI spec and should not be modified manually. Documentation improvements should be made in the OpenAPI spec files instead.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/car_action_controller.py:0-0
Timestamp: 2025-01-31T13:41:25.477Z
Learning: Files in fleet_management_api/controllers/ are auto-generated from the OpenAPI specification and should not be manually modified. Implementation of these methods should be done in the corresponding files under fleet_management_api/api_impl/ instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: fleet_management_api/controllers/route_controller.py:0-0
Timestamp: 2025-01-31T13:44:37.284Z
Learning: The controller files in `fleet_management_api/controllers/` are auto-generated and should not be modified manually. Changes should be made through the OpenAPI specification files instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/controllers/device_controller.py:0-0
Timestamp: 2025-05-12T07:29:43.317Z
Learning: The file `server/fleetv2_http_api/controllers/device_controller.py` is autogenerated and should not be modified manually.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#30
File: fleet_management_api/controllers/tenant_controller.py:11-23
Timestamp: 2025-04-22T12:06:06.987Z
Learning: Files under `fleet_management_api/controllers/` contain auto-generated code with placeholder implementations (like "do some magic!" return statements) and should not be flagged for issues like unused variables or placeholder return values.
source/bringauto/modules/mission_module/devices/AutonomyDevice.cpp (5)
Learnt from: MarioIvancik
PR: bringauto/virtual-vehicle#11
File: source/bringauto/communication/fleet_protocol/FleetProtocol.cpp:177-177
Timestamp: 2024-10-17T08:35:13.267Z
Learning: In the C++ file `source/bringauto/communication/fleet_protocol/FleetProtocol.cpp`, the method `status.getNextStop()` always returns an object with default-initialized members, so accessing its members without null checks is safe.
Learnt from: Mayt34
PR: bringauto/transparent-module#1
File: source/bringauto/modules/devices/testing_device/testing_module_manager.cpp:45-56
Timestamp: 2024-10-03T11:43:40.034Z
Learning: In the `testing_device_generate_first_command` function, adding `default_command` as a parameter may break other parts of the code, so it should not be added as a parameter.
Learnt from: Mayt34
PR: bringauto/transparent-module#1
File: source/bringauto/modules/devices/testing_device/testing_module_manager.cpp:45-56
Timestamp: 2024-10-09T05:03:50.249Z
Learning: In the `testing_device_generate_first_command` function, adding `default_command` as a parameter may break other parts of the code, so it should not be added as a parameter.
Learnt from: Mayt34
PR: bringauto/transparent-module#1
File: source/bringauto/modules/devices/testing_device/testing_module_manager.cpp:13-23
Timestamp: 2024-10-09T05:03:50.249Z
Learning: In the `testing_device_generate_command` function for the testing device, the command only depends on `current_command` generated by the device, similar to the io-module for button implementation.
Learnt from: Mayt34
PR: bringauto/transparent-module#1
File: source/bringauto/modules/devices/testing_device/testing_module_manager.cpp:13-23
Timestamp: 2024-10-03T11:16:43.026Z
Learning: In the `testing_device_generate_command` function for the testing device, the command only depends on `current_command` generated by the device, similar to the io-module for button implementation.
tests/README.md (7)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/mqtt/test_adapter.py:39-41
Timestamp: 2024-10-17T13:07:59.736Z
Learning: In `tests/mqtt/test_adapter.py` and other test files, prefer using fixed values for test configurations like `TEST_ADDRESS` and `TEST_PORT` instead of environment variables, to avoid requiring users to configure the environment for testing.
Learnt from: mikusaq
PR: bringauto/packager#24
File: README.md:0-0
Timestamp: 2024-10-09T05:03:50.249Z
Learning: The TODO section in `README.md` was outdated and has been removed.
Learnt from: mikusaq
PR: bringauto/packager#24
File: README.md:0-0
Timestamp: 2024-09-30T11:48:14.934Z
Learning: The TODO section in `README.md` was outdated and has been removed.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/database/test_db_operations.py:298-298
Timestamp: 2025-01-31T13:44:30.026Z
Learning: Tenant isolation tests are distributed across multiple test files in the codebase, including security/test_tenants.py, database/test_using_tenants.py, and controller-specific test files for cars, routes, and stops.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase has comprehensive tenant isolation testing across multiple test files:
1. Security tests in tests/security/test_tenants.py
2. Database tests in tests/database/test_using_tenants.py
3. Car-specific tenant isolation in tests/controllers/car/test_car_controller.py
4. API endpoint tests in tests/controllers/tenant/test_tenant.py
tests/libs/ExternalProtocol_pb2.py (5)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:1-25
Timestamp: 2024-10-14T06:44:03.705Z
Learning: In `external_server/adapters/api/adapter.py`, modifying `sys.path` to include a relative path to the Protobuf compiled modules (`lib/fleet-protocol/protobuf/compiled/python`) is acceptable because this location is expected to be stable and does not conflict with any other modules.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server_module/command_waiting_thread.py:6-6
Timestamp: 2024-10-15T12:07:47.529Z
Learning: In `external_server/server_module/command_waiting_thread.py` (Python), modifying `sys.path` at the module level is necessary to include the module `lib/fleet-protocol/protobuf/compiled/python`, which is nested deep inside a non-module folder structure.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/module_lib.py:7-7
Timestamp: 2024-10-14T07:00:50.683Z
Learning: In `external_server/adapters/api/module_lib.py`, modifying `sys.path` is necessary for importing modules from a non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:1-19
Timestamp: 2024-10-14T06:44:43.636Z
Learning: In this project, modifying `sys.path` is necessary for including protobuf modules from 'lib/fleet-protocol/protobuf/compiled/python' because they are nested deep inside a non-module folder structure.
tests/requirements.txt (11)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/mqtt/test_adapter.py:39-41
Timestamp: 2024-10-17T13:07:59.736Z
Learning: In `tests/mqtt/test_adapter.py` and other test files, prefer using fixed values for test configurations like `TEST_ADDRESS` and `TEST_PORT` instead of environment variables, to avoid requiring users to configure the environment for testing.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:1-19
Timestamp: 2024-10-14T06:44:43.636Z
Learning: In this project, modifying `sys.path` is necessary for including protobuf modules from 'lib/fleet-protocol/protobuf/compiled/python' because they are nested deep inside a non-module folder structure.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:1-25
Timestamp: 2024-10-14T06:44:03.705Z
Learning: In `external_server/adapters/api/adapter.py`, modifying `sys.path` to include a relative path to the Protobuf compiled modules (`lib/fleet-protocol/protobuf/compiled/python`) is acceptable because this location is expected to be stable and does not conflict with any other modules.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/models/payload.py:0-0
Timestamp: 2025-05-12T07:30:23.835Z
Learning: Files autogenerated by OpenAPI Generator in the fleet-protocol-http-api project (indicated by comments like "This class is auto generated by OpenAPI Generator") should not be manually modified, even if they contain linting issues or duplicate imports. Any changes should be made at the specification level and regenerated.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/models/module.py:0-0
Timestamp: 2025-05-12T07:30:17.701Z
Learning: Files in the server/fleetv2_http_api/models/ directory are auto-generated by OpenAPI Generator and should not be manually modified. Any necessary changes should be made to the OpenAPI specification instead.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase has comprehensive tenant isolation testing across multiple test files:
1. Security tests in tests/security/test_tenants.py
2. Database tests in tests/database/test_using_tenants.py
3. Car-specific tenant isolation in tests/controllers/car/test_car_controller.py
4. API endpoint tests in tests/controllers/tenant/test_tenant.py
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: tests_integration/request_validation/test_api_key_and_jwt_simultaneously.py:5-5
Timestamp: 2025-05-12T07:28:16.490Z
Learning: In the fleet-protocol-http-api project, all imports must be absolute and start from the root of the package, with "server" as the first part of the imported module name (e.g., "from server.app import get_test_app"), rather than starting with "fleetv2_http_api".
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server_module/command_waiting_thread.py:6-6
Timestamp: 2024-10-15T12:07:47.529Z
Learning: In `external_server/server_module/command_waiting_thread.py` (Python), modifying `sys.path` at the module level is necessary to include the module `lib/fleet-protocol/protobuf/compiled/python`, which is nested deep inside a non-module folder structure.
Learnt from: mikusaq
PR: bringauto/packager#24
File: example/package/fleet-protocol-internal-client/internal_client_debug.json:23-23
Timestamp: 2024-10-09T05:03:50.249Z
Learning: The repository URL for 'fleet-protocol-internal-client' is 'https://github.com/bringauto/internal-client-cpp.git', not 'https://github.com/bringauto/fleet-protocol-internal-client.git'.
tests/test_disconnect_on_stop_arrival.py (2)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
tests/libs/InternalProtocol_pb2.py (5)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:1-25
Timestamp: 2024-10-14T06:44:03.705Z
Learning: In `external_server/adapters/api/adapter.py`, modifying `sys.path` to include a relative path to the Protobuf compiled modules (`lib/fleet-protocol/protobuf/compiled/python`) is acceptable because this location is expected to be stable and does not conflict with any other modules.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server_module/command_waiting_thread.py:6-6
Timestamp: 2024-10-15T12:07:47.529Z
Learning: In `external_server/server_module/command_waiting_thread.py` (Python), modifying `sys.path` at the module level is necessary to include the module `lib/fleet-protocol/protobuf/compiled/python`, which is nested deep inside a non-module folder structure.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/fleet-protocol-http-api#29
File: server/fleetv2_http_api/controllers/device_controller.py:0-0
Timestamp: 2025-05-12T07:29:43.317Z
Learning: The file `server/fleetv2_http_api/controllers/device_controller.py` is autogenerated and should not be modified manually.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/module_lib.py:7-7
Timestamp: 2024-10-14T07:00:50.683Z
Learning: In `external_server/adapters/api/module_lib.py`, modifying `sys.path` is necessary for importing modules from a non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
tests/libs/testing_utilities.py (5)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/database/test_database_cleanup.py:53-54
Timestamp: 2025-01-31T13:43:48.677Z
Learning: In fleet-management-http-api tests, the database is completely recreated for each test using docker compose, making explicit cleanup of individual records unnecessary.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase has comprehensive tenant isolation testing across multiple test files:
1. Security tests in tests/security/test_tenants.py
2. Database tests in tests/database/test_using_tenants.py
3. Car-specific tenant isolation in tests/controllers/car/test_car_controller.py
4. API endpoint tests in tests/controllers/tenant/test_tenant.py
Learnt from: MarioIvancik
PR: bringauto/fleet-protocol-http-api#18
File: Dockerfile:16-17
Timestamp: 2024-10-24T12:17:01.106Z
Learning: In the `fleet-protocol-http-api` project, modifying the `ENTRYPOINT` and `CMD` in the `Dockerfile` as previously suggested leads to issues when running the image in Docker Compose.
tests/libs/MissionModule_pb2.py (8)
Learnt from: MarioIvancik
PR: bringauto/mission-module#19
File: source/external_server_api.cpp:132-134
Timestamp: 2024-10-23T09:20:29.194Z
Learning: In the mission module, Protobuf is retained for message validation because it works properly, but the creation of Protobuf data during parsing tends to contain bugs. Therefore, JSON is used for parsing while Protobuf handles validation.
Learnt from: MarioIvancik
PR: bringauto/mission-module#19
File: source/external_server_api.cpp:281-283
Timestamp: 2024-10-23T09:20:16.578Z
Learning: In the mission module codebase, Protobuf is used only for message validation because Protobuf validation works properly, but creating Protobuf data when parsing messages tends to contain bugs.
Learnt from: MarioIvancik
PR: bringauto/mission-module#19
File: CMakeLists.txt:13-13
Timestamp: 2024-10-23T09:33:50.060Z
Learning: In the mission module project, due to bugs in the protobuf library, messages are always sent as raw JSON, and protobuf is used solely for data validation.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server_module/command_waiting_thread.py:6-6
Timestamp: 2024-10-15T12:07:47.529Z
Learning: In `external_server/server_module/command_waiting_thread.py` (Python), modifying `sys.path` at the module level is necessary to include the module `lib/fleet-protocol/protobuf/compiled/python`, which is nested deep inside a non-module folder structure.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:1-19
Timestamp: 2024-10-14T06:44:43.636Z
Learning: In this project, modifying `sys.path` is necessary for including protobuf modules from 'lib/fleet-protocol/protobuf/compiled/python' because they are nested deep inside a non-module folder structure.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:1-25
Timestamp: 2024-10-14T06:44:03.705Z
Learning: In `external_server/adapters/api/adapter.py`, modifying `sys.path` to include a relative path to the Protobuf compiled modules (`lib/fleet-protocol/protobuf/compiled/python`) is acceptable because this location is expected to be stable and does not conflict with any other modules.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/module_lib.py:7-7
Timestamp: 2024-10-14T07:00:50.683Z
Learning: In `external_server/adapters/api/module_lib.py`, modifying `sys.path` is necessary for importing modules from a non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
tests/libs/sniff_mqtt.py (7)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/mqtt/test_adapter.py:39-41
Timestamp: 2024-10-17T13:07:59.736Z
Learning: In `tests/mqtt/test_adapter.py` and other test files, prefer using fixed values for test configurations like `TEST_ADDRESS` and `TEST_PORT` instead of environment variables, to avoid requiring users to configure the environment for testing.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:124-124
Timestamp: 2024-10-03T05:17:30.481Z
Learning: When using the `paho-mqtt` client, accessing the private attribute `_thread` of the MQTT client directly is acceptable if it provides a more reliable way to check if the loop is running, rather than maintaining a separate flag. If `_thread` is deprecated in future versions, implement an alternative mechanism.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:124-124
Timestamp: 2024-10-09T05:03:50.249Z
Learning: When using the `paho-mqtt` client, accessing the private attribute `_thread` of the MQTT client directly is acceptable if it provides a more reliable way to check if the loop is running, rather than maintaining a separate flag. If `_thread` is deprecated in future versions, implement an alternative mechanism.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:158-182
Timestamp: 2024-10-14T08:13:26.735Z
Learning: In the `CarServer` class, retries for connecting to the MQTT broker are implemented, so additional retry logic in `MQTTClientAdapter.connect` is unnecessary.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:76-78
Timestamp: 2024-10-14T06:49:18.563Z
Learning: In the external server Python codebase, configuration validation is module-specific and must be handled by the module implementation itself, not by adapter classes like `MQTTClientAdapter` or `APIClientAdapter`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server.py:0-0
Timestamp: 2024-10-15T07:55:43.454Z
Learning: In the `CarServer` class in `external_server/server.py`, if the connect message does not arrive in time, the connect sequence is repeated via other `CarServer` methods.
🧬 Code Graph Analysis (1)
tests/test_disconnect_on_stop_arrival.py (2)
tests/libs/testing_utilities.py (6)
run_docker_compose_all(18-19)check_car_state_and_next_stop(54-59)kill_docker_component(42-43)run_docker_compose_profiles(26-31)get_status_errors(62-71)stop_docker_compose_all(22-23)tests/libs/sniff_mqtt.py (6)
MqttMonitoring(27-211)connect(56-65)start(68-70)get_mission_module_statuses(143-150)send_mission_module_command(190-211)stop(73-76)
🪛 LanguageTool
tests/README.md
[typographical] ~2-~2: It seems that a comma is missing.
Context: # Mission Module Tests So far only manual tests are implemented. ## ...
(SO_COMMA)
[uncategorized] ~24-~24: Possible missing comma found.
Context: ...eaches the first stop. After roughly 2m 30s the car should reach the next stop auto...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~25-~25: A determiner appears to be missing. Consider inserting it.
Context: .... If it doesn't exist, the test fails. Path to the etna docker-compose file needs t...
(AI_EN_LECTOR_MISSING_DETERMINER)
[uncategorized] ~30-~30: Loose punctuation mark.
Context: ...etna docker-compose file. - --logging : enables logging of communication of mis...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 Pylint (3.3.7)
tests/libs/ExternalProtocol_pb2.py
[error] 25-25: Undefined variable '_EXTERNALSERVER'
(E0602)
[error] 26-26: Undefined variable '_EXTERNALSERVER'
(E0602)
[error] 27-27: Undefined variable '_EXTERNALCLIENT'
(E0602)
[error] 28-28: Undefined variable '_EXTERNALCLIENT'
(E0602)
[error] 29-29: Undefined variable '_CONNECT'
(E0602)
[error] 30-30: Undefined variable '_CONNECT'
(E0602)
[error] 31-31: Undefined variable '_CONNECTRESPONSE'
(E0602)
[error] 32-32: Undefined variable '_CONNECTRESPONSE'
(E0602)
[error] 33-33: Undefined variable '_CONNECTRESPONSE_TYPE'
(E0602)
[error] 34-34: Undefined variable '_CONNECTRESPONSE_TYPE'
(E0602)
[error] 35-35: Undefined variable '_STATUS'
(E0602)
[error] 36-36: Undefined variable '_STATUS'
(E0602)
[error] 37-37: Undefined variable '_STATUS_DEVICESTATE'
(E0602)
[error] 38-38: Undefined variable '_STATUS_DEVICESTATE'
(E0602)
[error] 39-39: Undefined variable '_STATUSRESPONSE'
(E0602)
[error] 40-40: Undefined variable '_STATUSRESPONSE'
(E0602)
[error] 41-41: Undefined variable '_STATUSRESPONSE_TYPE'
(E0602)
[error] 42-42: Undefined variable '_STATUSRESPONSE_TYPE'
(E0602)
[error] 43-43: Undefined variable '_COMMAND'
(E0602)
[error] 44-44: Undefined variable '_COMMAND'
(E0602)
[error] 45-45: Undefined variable '_COMMANDRESPONSE'
(E0602)
[error] 46-46: Undefined variable '_COMMANDRESPONSE'
(E0602)
[error] 47-47: Undefined variable '_COMMANDRESPONSE_TYPE'
(E0602)
[error] 48-48: Undefined variable '_COMMANDRESPONSE_TYPE'
(E0602)
tests/test_disconnect_on_stop_arrival.py
[error] 14-14: Class 'AutonomyCommand' has no 'Action' member
(E1101)
[error] 24-24: Instance of 'AutonomyCommand' has no 'stops' member
(E1101)
[refactor] 37-37: Consider using 'sys.exit' instead
(R1722)
tests/libs/InternalProtocol_pb2.py
[error] 24-24: Undefined variable '_INTERNALCLIENT'
(E0602)
[error] 25-25: Undefined variable '_INTERNALCLIENT'
(E0602)
[error] 26-26: Undefined variable '_INTERNALSERVER'
(E0602)
[error] 27-27: Undefined variable '_INTERNALSERVER'
(E0602)
[error] 28-28: Undefined variable '_DEVICECONNECT'
(E0602)
[error] 29-29: Undefined variable '_DEVICECONNECT'
(E0602)
[error] 30-30: Undefined variable '_DEVICECONNECTRESPONSE'
(E0602)
[error] 31-31: Undefined variable '_DEVICECONNECTRESPONSE'
(E0602)
[error] 32-32: Undefined variable '_DEVICECONNECTRESPONSE_RESPONSETYPE'
(E0602)
[error] 33-33: Undefined variable '_DEVICECONNECTRESPONSE_RESPONSETYPE'
(E0602)
[error] 34-34: Undefined variable '_DEVICESTATUS'
(E0602)
[error] 35-35: Undefined variable '_DEVICESTATUS'
(E0602)
[error] 36-36: Undefined variable '_DEVICECOMMAND'
(E0602)
[error] 37-37: Undefined variable '_DEVICECOMMAND'
(E0602)
[error] 38-38: Undefined variable '_DEVICE'
(E0602)
[error] 39-39: Undefined variable '_DEVICE'
(E0602)
[error] 40-40: Undefined variable '_DEVICE_MODULE'
(E0602)
[error] 41-41: Undefined variable '_DEVICE_MODULE'
(E0602)
tests/libs/sniff_mqtt.py
[refactor] 27-27: Too many instance attributes (13/7)
(R0902)
[refactor] 79-79: Too many arguments (6/5)
(R0913)
[refactor] 79-79: Too many positional arguments (6/5)
(R0917)
🪛 Flake8 (7.2.0)
tests/libs/ExternalProtocol_pb2.py
[error] 14-14: 'libs.InternalProtocol_pb2 as InternalProtocol__pb2' imported but unused
(F401)
[error] 14-14: module level import not at top of file
(E402)
[error] 21-21: comparison to False should be 'if cond is False:' or 'if not cond:'
(E712)
[error] 23-23: indentation is not a multiple of 4
(E111)
[error] 24-24: indentation is not a multiple of 4
(E111)
[error] 25-25: undefined name '_EXTERNALSERVER'
(F821)
[error] 25-25: indentation is not a multiple of 4
(E111)
[error] 25-25: missing whitespace around operator
(E225)
[error] 26-26: undefined name '_EXTERNALSERVER'
(F821)
[error] 26-26: indentation is not a multiple of 4
(E111)
[error] 26-26: missing whitespace around operator
(E225)
[error] 27-27: undefined name '_EXTERNALCLIENT'
(F821)
[error] 27-27: indentation is not a multiple of 4
(E111)
[error] 27-27: missing whitespace around operator
(E225)
[error] 28-28: undefined name '_EXTERNALCLIENT'
(F821)
[error] 28-28: indentation is not a multiple of 4
(E111)
[error] 28-28: missing whitespace around operator
(E225)
[error] 29-29: undefined name '_CONNECT'
(F821)
[error] 29-29: indentation is not a multiple of 4
(E111)
[error] 29-29: missing whitespace around operator
(E225)
[error] 30-30: undefined name '_CONNECT'
(F821)
[error] 30-30: indentation is not a multiple of 4
(E111)
[error] 30-30: missing whitespace around operator
(E225)
[error] 31-31: undefined name '_CONNECTRESPONSE'
(F821)
[error] 31-31: indentation is not a multiple of 4
(E111)
[error] 31-31: missing whitespace around operator
(E225)
[error] 32-32: undefined name '_CONNECTRESPONSE'
(F821)
[error] 32-32: indentation is not a multiple of 4
(E111)
[error] 32-32: missing whitespace around operator
(E225)
[error] 33-33: undefined name '_CONNECTRESPONSE_TYPE'
(F821)
[error] 33-33: indentation is not a multiple of 4
(E111)
[error] 33-33: missing whitespace around operator
(E225)
[error] 34-34: undefined name '_CONNECTRESPONSE_TYPE'
(F821)
[error] 34-34: indentation is not a multiple of 4
(E111)
[error] 34-34: missing whitespace around operator
(E225)
[error] 35-35: undefined name '_STATUS'
(F821)
[error] 35-35: indentation is not a multiple of 4
(E111)
[error] 35-35: missing whitespace around operator
(E225)
[error] 36-36: undefined name '_STATUS'
(F821)
[error] 36-36: indentation is not a multiple of 4
(E111)
[error] 36-36: missing whitespace around operator
(E225)
[error] 37-37: undefined name '_STATUS_DEVICESTATE'
(F821)
[error] 37-37: indentation is not a multiple of 4
(E111)
[error] 37-37: missing whitespace around operator
(E225)
[error] 38-38: undefined name '_STATUS_DEVICESTATE'
(F821)
[error] 38-38: indentation is not a multiple of 4
(E111)
[error] 38-38: missing whitespace around operator
(E225)
[error] 39-39: undefined name '_STATUSRESPONSE'
(F821)
[error] 39-39: indentation is not a multiple of 4
(E111)
[error] 39-39: missing whitespace around operator
(E225)
[error] 40-40: undefined name '_STATUSRESPONSE'
(F821)
[error] 40-40: indentation is not a multiple of 4
(E111)
[error] 40-40: missing whitespace around operator
(E225)
[error] 41-41: undefined name '_STATUSRESPONSE_TYPE'
(F821)
[error] 41-41: indentation is not a multiple of 4
(E111)
[error] 41-41: missing whitespace around operator
(E225)
[error] 42-42: undefined name '_STATUSRESPONSE_TYPE'
(F821)
[error] 42-42: indentation is not a multiple of 4
(E111)
[error] 42-42: missing whitespace around operator
(E225)
[error] 43-43: undefined name '_COMMAND'
(F821)
[error] 43-43: indentation is not a multiple of 4
(E111)
[error] 43-43: missing whitespace around operator
(E225)
[error] 44-44: undefined name '_COMMAND'
(F821)
[error] 44-44: indentation is not a multiple of 4
(E111)
[error] 44-44: missing whitespace around operator
(E225)
[error] 45-45: undefined name '_COMMANDRESPONSE'
(F821)
[error] 45-45: indentation is not a multiple of 4
(E111)
[error] 45-45: missing whitespace around operator
(E225)
[error] 46-46: undefined name '_COMMANDRESPONSE'
(F821)
[error] 46-46: indentation is not a multiple of 4
(E111)
[error] 46-46: missing whitespace around operator
(E225)
[error] 47-47: undefined name '_COMMANDRESPONSE_TYPE'
(F821)
[error] 47-47: indentation is not a multiple of 4
(E111)
[error] 47-47: missing whitespace around operator
(E225)
[error] 48-48: undefined name '_COMMANDRESPONSE_TYPE'
(F821)
[error] 48-48: indentation is not a multiple of 4
(E111)
[error] 48-48: missing whitespace around operator
(E225)
tests/test_disconnect_on_stop_arrival.py
[error] 11-11: too many blank lines (3)
(E303)
[error] 20-20: continuation line over-indented for visual indent
(E127)
[error] 23-23: continuation line over-indented for visual indent
(E127)
[error] 58-58: block comment should start with '# '
(E265)
[error] 76-76: block comment should start with '# '
(E265)
[error] 89-89: too many blank lines (2)
(E303)
tests/libs/InternalProtocol_pb2.py
[error] 16-16: too many blank lines (4)
(E303)
[error] 20-20: comparison to False should be 'if cond is False:' or 'if not cond:'
(E712)
[error] 22-22: indentation is not a multiple of 4
(E111)
[error] 23-23: indentation is not a multiple of 4
(E111)
[error] 24-24: undefined name '_INTERNALCLIENT'
(F821)
[error] 24-24: indentation is not a multiple of 4
(E111)
[error] 24-24: missing whitespace around operator
(E225)
[error] 25-25: undefined name '_INTERNALCLIENT'
(F821)
[error] 25-25: indentation is not a multiple of 4
(E111)
[error] 25-25: missing whitespace around operator
(E225)
[error] 26-26: undefined name '_INTERNALSERVER'
(F821)
[error] 26-26: indentation is not a multiple of 4
(E111)
[error] 26-26: missing whitespace around operator
(E225)
[error] 27-27: undefined name '_INTERNALSERVER'
(F821)
[error] 27-27: indentation is not a multiple of 4
(E111)
[error] 27-27: missing whitespace around operator
(E225)
[error] 28-28: undefined name '_DEVICECONNECT'
(F821)
[error] 28-28: indentation is not a multiple of 4
(E111)
[error] 28-28: missing whitespace around operator
(E225)
[error] 29-29: undefined name '_DEVICECONNECT'
(F821)
[error] 29-29: indentation is not a multiple of 4
(E111)
[error] 29-29: missing whitespace around operator
(E225)
[error] 30-30: undefined name '_DEVICECONNECTRESPONSE'
(F821)
[error] 30-30: indentation is not a multiple of 4
(E111)
[error] 30-30: missing whitespace around operator
(E225)
[error] 31-31: undefined name '_DEVICECONNECTRESPONSE'
(F821)
[error] 31-31: indentation is not a multiple of 4
(E111)
[error] 31-31: missing whitespace around operator
(E225)
[error] 32-32: undefined name '_DEVICECONNECTRESPONSE_RESPONSETYPE'
(F821)
[error] 32-32: indentation is not a multiple of 4
(E111)
[error] 32-32: missing whitespace around operator
(E225)
[error] 33-33: undefined name '_DEVICECONNECTRESPONSE_RESPONSETYPE'
(F821)
[error] 33-33: indentation is not a multiple of 4
(E111)
[error] 33-33: missing whitespace around operator
(E225)
[error] 34-34: undefined name '_DEVICESTATUS'
(F821)
[error] 34-34: indentation is not a multiple of 4
(E111)
[error] 34-34: missing whitespace around operator
(E225)
[error] 35-35: undefined name '_DEVICESTATUS'
(F821)
[error] 35-35: indentation is not a multiple of 4
(E111)
[error] 35-35: missing whitespace around operator
(E225)
[error] 36-36: undefined name '_DEVICECOMMAND'
(F821)
[error] 36-36: indentation is not a multiple of 4
(E111)
[error] 36-36: missing whitespace around operator
(E225)
[error] 37-37: undefined name '_DEVICECOMMAND'
(F821)
[error] 37-37: indentation is not a multiple of 4
(E111)
[error] 37-37: missing whitespace around operator
(E225)
[error] 38-38: undefined name '_DEVICE'
(F821)
[error] 38-38: indentation is not a multiple of 4
(E111)
[error] 38-38: missing whitespace around operator
(E225)
[error] 39-39: undefined name '_DEVICE'
(F821)
[error] 39-39: indentation is not a multiple of 4
(E111)
[error] 39-39: missing whitespace around operator
(E225)
[error] 40-40: undefined name '_DEVICE_MODULE'
(F821)
[error] 40-40: indentation is not a multiple of 4
(E111)
[error] 40-40: missing whitespace around operator
(E225)
[error] 41-41: undefined name '_DEVICE_MODULE'
(F821)
[error] 41-41: indentation is not a multiple of 4
(E111)
[error] 41-41: missing whitespace around operator
(E225)
tests/libs/testing_utilities.py
[error] 2-2: at least two spaces before inline comment
(E261)
[error] 6-6: too many blank lines (3)
(E303)
[error] 15-15: whitespace before ']'
(E202)
tests/libs/MissionModule_pb2.py
[error] 16-16: too many blank lines (4)
(E303)
[error] 27-27: too many blank lines (3)
(E303)
[error] 188-188: continuation line missing indentation or outdented
(E122)
[error] 381-381: whitespace before ':'
(E203)
[error] 382-382: whitespace before ':'
(E203)
[error] 383-383: whitespace before ':'
(E203)
[error] 385-385: whitespace before ','
(E203)
[error] 387-387: whitespace before ':'
(E203)
[error] 388-388: whitespace before ':'
(E203)
[error] 395-395: whitespace before ':'
(E203)
[error] 396-396: whitespace before ':'
(E203)
[error] 402-402: whitespace before ':'
(E203)
[error] 403-403: whitespace before ':'
(E203)
[error] 409-409: whitespace before ':'
(E203)
[error] 410-410: whitespace before ':'
(E203)
[error] 416-416: whitespace before ':'
(E203)
[error] 417-417: whitespace before ':'
(E203)
tests/libs/sniff_mqtt.py
[error] 1-1: multiple imports on one line
(E401)
[error] 14-14: too many blank lines (3)
(E303)
[error] 44-44: too many blank lines (2)
(E303)
[error] 45-45: missing whitespace around operator
(E225)
[error] 56-56: too many blank lines (2)
(E303)
[error] 68-68: too many blank lines (2)
(E303)
[error] 73-73: too many blank lines (2)
(E303)
[error] 79-79: too many blank lines (2)
(E303)
[error] 83-83: too many blank lines (2)
(E303)
[error] 84-84: multiple statements on one line (colon)
(E701)
[error] 96-96: too many blank lines (2)
(E303)
[error] 106-106: f-string is missing placeholders
(F541)
[error] 107-107: multiple statements on one line (colon)
(E701)
[error] 110-110: too many blank lines (2)
(E303)
[error] 120-120: f-string is missing placeholders
(F541)
[error] 121-121: multiple statements on one line (colon)
(E701)
[error] 124-124: too many blank lines (2)
(E303)
[error] 131-131: too many blank lines (2)
(E303)
[error] 143-143: too many blank lines (2)
(E303)
[error] 153-153: too many blank lines (2)
(E303)
[error] 162-162: too many blank lines (2)
(E303)
[error] 174-174: too many blank lines (2)
(E303)
[error] 184-184: too many blank lines (2)
(E303)
[error] 190-190: too many blank lines (2)
(E303)
[error] 208-208: f-string is missing placeholders
(F541)
[error] 215-215: too many blank lines (3)
(E303)
🪛 Ruff (0.11.9)
tests/libs/ExternalProtocol_pb2.py
14-14: libs.InternalProtocol_pb2 imported but unused
Remove unused import: libs.InternalProtocol_pb2
(F401)
21-21: Avoid equality comparisons to False; use if not _descriptor._USE_C_DESCRIPTORS: for false checks
Replace with not _descriptor._USE_C_DESCRIPTORS
(E712)
25-25: Undefined name _EXTERNALSERVER
(F821)
26-26: Undefined name _EXTERNALSERVER
(F821)
27-27: Undefined name _EXTERNALCLIENT
(F821)
28-28: Undefined name _EXTERNALCLIENT
(F821)
29-29: Undefined name _CONNECT
(F821)
30-30: Undefined name _CONNECT
(F821)
31-31: Undefined name _CONNECTRESPONSE
(F821)
32-32: Undefined name _CONNECTRESPONSE
(F821)
33-33: Undefined name _CONNECTRESPONSE_TYPE
(F821)
34-34: Undefined name _CONNECTRESPONSE_TYPE
(F821)
35-35: Undefined name _STATUS
(F821)
36-36: Undefined name _STATUS
(F821)
37-37: Undefined name _STATUS_DEVICESTATE
(F821)
38-38: Undefined name _STATUS_DEVICESTATE
(F821)
39-39: Undefined name _STATUSRESPONSE
(F821)
40-40: Undefined name _STATUSRESPONSE
(F821)
41-41: Undefined name _STATUSRESPONSE_TYPE
(F821)
42-42: Undefined name _STATUSRESPONSE_TYPE
(F821)
43-43: Undefined name _COMMAND
(F821)
44-44: Undefined name _COMMAND
(F821)
45-45: Undefined name _COMMANDRESPONSE
(F821)
46-46: Undefined name _COMMANDRESPONSE
(F821)
47-47: Undefined name _COMMANDRESPONSE_TYPE
(F821)
48-48: Undefined name _COMMANDRESPONSE_TYPE
(F821)
tests/libs/InternalProtocol_pb2.py
20-20: Avoid equality comparisons to False; use if not _descriptor._USE_C_DESCRIPTORS: for false checks
Replace with not _descriptor._USE_C_DESCRIPTORS
(E712)
24-24: Undefined name _INTERNALCLIENT
(F821)
25-25: Undefined name _INTERNALCLIENT
(F821)
26-26: Undefined name _INTERNALSERVER
(F821)
27-27: Undefined name _INTERNALSERVER
(F821)
28-28: Undefined name _DEVICECONNECT
(F821)
29-29: Undefined name _DEVICECONNECT
(F821)
30-30: Undefined name _DEVICECONNECTRESPONSE
(F821)
31-31: Undefined name _DEVICECONNECTRESPONSE
(F821)
32-32: Undefined name _DEVICECONNECTRESPONSE_RESPONSETYPE
(F821)
33-33: Undefined name _DEVICECONNECTRESPONSE_RESPONSETYPE
(F821)
34-34: Undefined name _DEVICESTATUS
(F821)
35-35: Undefined name _DEVICESTATUS
(F821)
36-36: Undefined name _DEVICECOMMAND
(F821)
37-37: Undefined name _DEVICECOMMAND
(F821)
38-38: Undefined name _DEVICE
(F821)
39-39: Undefined name _DEVICE
(F821)
40-40: Undefined name _DEVICE_MODULE
(F821)
41-41: Undefined name _DEVICE_MODULE
(F821)
tests/libs/testing_utilities.py
48-51: Use return any(status.state == state for status in statuses) instead of for loop
Replace with return any(status.state == state for status in statuses)
(SIM110)
tests/libs/sniff_mqtt.py
84-84: Multiple statements on one line (colon)
(E701)
100-100: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
106-106: f-string without any placeholders
Remove extraneous f prefix
(F541)
107-107: Multiple statements on one line (colon)
(E701)
114-114: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
120-120: f-string without any placeholders
Remove extraneous f prefix
(F541)
121-121: Multiple statements on one line (colon)
(E701)
208-208: f-string without any placeholders
Remove extraneous f prefix
(F541)
🪛 Gitleaks (8.26.0)
tests/libs/MissionModule_pb2.py
21-22: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (7)
CMakeLists.txt (1)
13-13: Version bump acknowledged – remember to tag & update package metadata.
MISSION_MODULE_VERSIONis now1.2.13. Ensure the corresponding Git tag and any downstream manifest (Docker image labels, package.json, etc.) are updated in the release process.source/bringauto/modules/mission_module/devices/AutonomyDevice.cpp (1)
41-48: Enhanced stop removal logic looks correct.The broadened condition for removing the first stop makes sense for handling edge cases, particularly MQTT disconnection scenarios. The logic now correctly handles both the normal state transition (DRIVE → IN_STOP) and the case where the vehicle is already at a stop that matches the next stop name.
tests/README.md (1)
1-32: Well-structured test documentation.The README provides clear setup instructions and explains the test scenario effectively. The manual test description is comprehensive and will help users understand both the setup requirements and expected behavior.
tests/test_disconnect_on_stop_arrival.py (1)
17-25: Well-implemented integration test.The test effectively simulates the MQTT disconnection scenario and validates the expected behavior. The use of threading for MQTT monitoring, proper cleanup in exception handling, and verification of finished stops demonstrates good test design practices.
Also applies to: 54-97
tests/libs/ExternalProtocol_pb2.py (1)
1-49: Auto-generated protobuf bindings - no manual changes needed.This is a correctly generated protobuf file that provides Python bindings for ExternalProtocol messages. The static analysis warnings can be safely ignored as they're false positives for auto-generated code. The protobuf compiler correctly generates the descriptor variables and message classes.
tests/libs/InternalProtocol_pb2.py (1)
1-42: Auto-generated protobuf bindings - correctly implemented.This auto-generated file provides Python bindings for InternalProtocol messages used by the testing infrastructure. Like other protobuf generated files, the static analysis warnings are false positives and can be ignored. The file correctly defines the message classes needed for MQTT communication testing.
tests/libs/MissionModule_pb2.py (1)
1-425: Skip review of auto-generated protobuf file.This file is auto-generated by the protocol buffer compiler and should not be manually edited. Any formatting or style issues in this file are expected and will be regenerated from the source
.protofile.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/libs/testing_utilities.py (1)
42-43: Function name correctly updated.Good to see the function name was updated from
kill_docker_componenttostop_docker_componentto accurately reflect its behavior of gracefully stopping containers.
🧹 Nitpick comments (7)
tests/test_disconnect_on_stop_arrival.py (3)
42-42: Consider making the component name construction more robust.The current string splitting approach is fragile and depends on a specific directory structure. Consider using a more explicit approach or adding validation.
- mqtt_component_name = str(args.yaml).split('/')[-2] + '-vernemq-1' + # Extract directory name more safely + yaml_dir = os.path.dirname(args.yaml) + dir_name = os.path.basename(yaml_dir) if yaml_dir else 'default' + mqtt_component_name = f'{dir_name}-vernemq-1'
60-60: Remove commented debug code.Clean up the commented print statements that were used for debugging.
- #print(statuses)- #print(statuses)Also applies to: 78-78
82-82: Consider using structured access instead ofto_dict().Direct access to the internal dictionary structure via
to_dict()might be fragile. Consider using proper protobuf field access if available.tests/libs/testing_utilities.py (1)
46-51: Simplify usingany()for better readability.The static analysis correctly identifies that this loop can be simplified using the
any()built-in function.-def check_car_state(statuses: list, state: int) -> bool: - """Check if any of the statuses has the given state.""" - for status in statuses: - if status.state == state: - return True - return False +def check_car_state(statuses: list, state: int) -> bool: + """Check if any of the statuses has the given state.""" + return any(status.state == state for status in statuses)tests/libs/sniff_mqtt.py (3)
84-84: Fix code style: avoid multiple statements on one line.Separate the conditional statements onto multiple lines for better readability.
- if self._verbose: self._print(f'topic: {message.topic}') + if self._verbose: + self._print(f'topic: {message.topic}')- if self._verbose: self._print(message) + if self._verbose: + self._print(message)- if self._verbose: self._print(message) + if self._verbose: + self._print(message)Also applies to: 107-107, 121-121
100-100: Use direct attribute access instead ofgetattrwith constants.Replace
getattrcalls with direct attribute access for better performance and clarity.- self._handle_status(getattr(message, 'status').deviceStatus) + self._handle_status(message.status.deviceStatus)- self._handle_command(getattr(message, 'command')) + self._handle_command(message.command)Also applies to: 114-114
45-45: Fix spacing in Client constructor.Add a space after the opening parenthesis for consistent formatting.
- self._client =Client( + self._client = Client(
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/libs/sniff_mqtt.py(1 hunks)tests/libs/testing_utilities.py(1 hunks)tests/requirements.txt(1 hunks)tests/test_disconnect_on_stop_arrival.py(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/requirements.txt
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/mqtt/test_adapter.py:39-41
Timestamp: 2024-10-17T13:07:59.736Z
Learning: In `tests/mqtt/test_adapter.py` and other test files, prefer using fixed values for test configurations like `TEST_ADDRESS` and `TEST_PORT` instead of environment variables, to avoid requiring users to configure the environment for testing.
tests/test_disconnect_on_stop_arrival.py (2)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
tests/libs/sniff_mqtt.py (9)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/mqtt/test_adapter.py:39-41
Timestamp: 2024-10-17T13:07:59.736Z
Learning: In `tests/mqtt/test_adapter.py` and other test files, prefer using fixed values for test configurations like `TEST_ADDRESS` and `TEST_PORT` instead of environment variables, to avoid requiring users to configure the environment for testing.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:124-124
Timestamp: 2024-10-03T05:17:30.481Z
Learning: When using the `paho-mqtt` client, accessing the private attribute `_thread` of the MQTT client directly is acceptable if it provides a more reliable way to check if the loop is running, rather than maintaining a separate flag. If `_thread` is deprecated in future versions, implement an alternative mechanism.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:124-124
Timestamp: 2024-10-09T05:03:50.249Z
Learning: When using the `paho-mqtt` client, accessing the private attribute `_thread` of the MQTT client directly is acceptable if it provides a more reliable way to check if the loop is running, rather than maintaining a separate flag. If `_thread` is deprecated in future versions, implement an alternative mechanism.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server_module/command_waiting_thread.py:6-6
Timestamp: 2024-10-15T12:07:47.529Z
Learning: In `external_server/server_module/command_waiting_thread.py` (Python), modifying `sys.path` at the module level is necessary to include the module `lib/fleet-protocol/protobuf/compiled/python`, which is nested deep inside a non-module folder structure.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:158-182
Timestamp: 2024-10-14T08:13:26.735Z
Learning: In the `CarServer` class, retries for connecting to the MQTT broker are implemented, so additional retry logic in `MQTTClientAdapter.connect` is unnecessary.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:76-78
Timestamp: 2024-10-14T06:49:18.563Z
Learning: In the external server Python codebase, configuration validation is module-specific and must be handled by the module implementation itself, not by adapter classes like `MQTTClientAdapter` or `APIClientAdapter`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server.py:0-0
Timestamp: 2024-10-15T07:55:43.454Z
Learning: In the `CarServer` class in `external_server/server.py`, if the connect message does not arrive in time, the connect sequence is repeated via other `CarServer` methods.
Learnt from: MarioIvancik
PR: bringauto/module-gateway#39
File: source/bringauto/external_client/connection/ExternalConnection.cpp:309-313
Timestamp: 2025-01-07T12:43:09.880Z
Learning: In the ModuleGateway project, the `toExternalQueue_` in ModuleHandler is used for sending messages to the external server and needs to be monitored to prevent deadlocks, while `commandQueue_` is used for a different purpose of handling device commands.
tests/libs/testing_utilities.py (5)
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/database/test_database_cleanup.py:53-54
Timestamp: 2025-01-31T13:43:48.677Z
Learning: In fleet-management-http-api tests, the database is completely recreated for each test using docker compose, making explicit cleanup of individual records unnecessary.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase has comprehensive tenant isolation testing across multiple test files:
1. Security tests in tests/security/test_tenants.py
2. Database tests in tests/database/test_using_tenants.py
3. Car-specific tenant isolation in tests/controllers/car/test_car_controller.py
4. API endpoint tests in tests/controllers/tenant/test_tenant.py
Learnt from: MarioIvancik
PR: bringauto/fleet-protocol-http-api#18
File: Dockerfile:16-17
Timestamp: 2024-10-24T12:17:01.106Z
Learning: In the `fleet-protocol-http-api` project, modifying the `ENTRYPOINT` and `CMD` in the `Dockerfile` as previously suggested leads to issues when running the image in Docker Compose.
🪛 Ruff (0.11.9)
tests/libs/sniff_mqtt.py
84-84: Multiple statements on one line (colon)
(E701)
100-100: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
107-107: Multiple statements on one line (colon)
(E701)
114-114: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
121-121: Multiple statements on one line (colon)
(E701)
tests/libs/testing_utilities.py
48-51: Use return any(status.state == state for status in statuses) instead of for loop
Replace with return any(status.state == state for status in statuses)
(SIM110)
🔇 Additional comments (7)
tests/test_disconnect_on_stop_arrival.py (2)
11-25: Well-structured command creation function.The function correctly creates an autonomy command with proper stop definitions and GPS coordinates. The logic is clear and follows good practices.
92-105: Excellent error handling and cleanup.The exception handling and cleanup logic properly handles both keyboard interrupts and general exceptions, ensuring Docker services are stopped and the test result is clearly communicated.
tests/libs/testing_utilities.py (2)
64-70: Fixed values for test configuration align with best practices.The use of fixed values for
host,company_name, andcar_namefollows the learned preference for test configurations, avoiding the need for users to configure environment variables.
14-39: Well-structured Docker Compose utility functions.The functions provide a clean abstraction for managing Docker Compose services with proper profile support and consistent command building.
tests/libs/sniff_mqtt.py (3)
56-66: Well-implemented connection logic with retry.The connection method properly handles connection failures with retry logic and uses fixed host/port values which aligns with testing best practices.
134-140: Excellent thread safety implementation.The use of condition variables and locks ensures thread-safe access to shared status and command collections. The message capping at 100 entries prevents memory issues during long-running tests.
Also applies to: 165-171
190-211: Robust command sending implementation.The method properly validates input types, constructs the protobuf message hierarchy correctly, and publishes with appropriate QoS settings.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
tests/test_disconnect_on_stop_arrival.py (1)
17-17: Past issue already resolved.The type hint syntax issue mentioned in previous reviews has been correctly fixed. The current syntax
list[libs.MissionModule_pb2.Station]is correct.
🧹 Nitpick comments (2)
tests/test_disconnect_on_stop_arrival.py (2)
60-60: Remove commented debug statements.Clean up the commented print statements that were likely used for debugging.
- #print(statuses) + # Wait until the car reaches the Svatopluka Čecha A stop.- #print(statuses) + # Check if upon reconnection the car already arrived at the Těšínská stop.Also applies to: 78-78
68-69: Consider making the wait time configurable.The hardcoded 2 minutes 45 seconds wait time might not be suitable for all test environments. Consider making this configurable through command-line arguments.
+ parser.add_argument('--wait-time', type=int, default=165, help='Wait time in seconds for car to arrive at next stop (default: 165).')Then use
args.wait_timeinstead of the hardcoded value.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/test_disconnect_on_stop_arrival.py(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/mqtt/test_adapter.py:39-41
Timestamp: 2024-10-17T13:07:59.736Z
Learning: In `tests/mqtt/test_adapter.py` and other test files, prefer using fixed values for test configurations like `TEST_ADDRESS` and `TEST_PORT` instead of environment variables, to avoid requiring users to configure the environment for testing.
tests/test_disconnect_on_stop_arrival.py (2)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
🔇 Additional comments (1)
tests/test_disconnect_on_stop_arrival.py (1)
28-105: Overall test structure looks good.The test properly handles setup, execution, cleanup, and provides clear pass/fail feedback. The exception handling and cleanup logic ensure resources are properly released.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
tests/libs/sniff_mqtt.py (7)
56-66: Consider optimization: avoid recreating client on each retry.The current implementation recreates the MQTT client on every connection attempt, which may be unnecessary overhead. Consider only recreating the client if the connection has been explicitly closed or failed.
def connect(self, host: str = '127.0.0.1', port: int = 1883, keepalive: int = 5) -> None: """Connects to the MQTT broker with retry logic.""" connected = False while not connected: try: - self._create_client() + if not hasattr(self, '_client') or not self._client.is_connected(): + self._create_client() self._client.connect(host=host, port=port, keepalive=keepalive) connected = True except ConnectionRefusedError: time.sleep(2)
84-84: Fix code style: separate statements onto different lines.Multiple statements on one line reduces readability.
- if self._verbose: self._print(f'topic: {message.topic}') + if self._verbose: + self._print(f'topic: {message.topic}')
100-100: Replace getattr with direct attribute access.Using
getattrwith a constant attribute name is unnecessary and less readable than direct property access.- self._handle_status(getattr(message, 'status').deviceStatus) + self._handle_status(message.status.deviceStatus)
107-107: Fix code style: separate statements onto different lines.Multiple statements on one line reduces readability.
- if self._verbose: self._print(message) + if self._verbose: + self._print(message)
114-114: Replace getattr with direct attribute access.Using
getattrwith a constant attribute name is unnecessary and less readable than direct property access.- self._handle_command(getattr(message, 'command')) + self._handle_command(message.command)
121-121: Fix code style: separate statements onto different lines.Multiple statements on one line reduces readability.
- if self._verbose: self._print(message) + if self._verbose: + self._print(message)
56-56: Consider using fixed test configuration values.Based on the retrieved learnings, prefer using fixed values for test configurations instead of default parameters to avoid requiring users to configure the environment for testing.
- def connect(self, host: str = '127.0.0.1', port: int = 1883, keepalive: int = 5) -> None: + def connect(self, host: str = '127.0.0.1', port: int = 1883, keepalive: int = 5) -> None:Note: The current implementation already uses fixed default values, which aligns with the learning. No change needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
lib/protobuf-mission-module/.gitignore(1 hunks)lib/protobuf-mission-module/mission_module_protobuf_files/MissionModule_pb2.py(1 hunks)lib/protobuf-mission-module/pyproject.toml(1 hunks)tests/libs/sniff_mqtt.py(1 hunks)tests/requirements.txt(1 hunks)tests/test_disconnect_on_stop_arrival.py(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- lib/protobuf-mission-module/pyproject.toml
- lib/protobuf-mission-module/.gitignore
- lib/protobuf-mission-module/mission_module_protobuf_files/MissionModule_pb2.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_disconnect_on_stop_arrival.py
- tests/requirements.txt
🧰 Additional context used
🧠 Learnings (1)
tests/libs/sniff_mqtt.py (8)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/mqtt/test_adapter.py:39-41
Timestamp: 2024-10-17T13:07:59.736Z
Learning: In `tests/mqtt/test_adapter.py` and other test files, prefer using fixed values for test configurations like `TEST_ADDRESS` and `TEST_PORT` instead of environment variables, to avoid requiring users to configure the environment for testing.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:124-124
Timestamp: 2024-10-03T05:17:30.481Z
Learning: When using the `paho-mqtt` client, accessing the private attribute `_thread` of the MQTT client directly is acceptable if it provides a more reliable way to check if the loop is running, rather than maintaining a separate flag. If `_thread` is deprecated in future versions, implement an alternative mechanism.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:124-124
Timestamp: 2024-10-09T05:03:50.249Z
Learning: When using the `paho-mqtt` client, accessing the private attribute `_thread` of the MQTT client directly is acceptable if it provides a more reliable way to check if the loop is running, rather than maintaining a separate flag. If `_thread` is deprecated in future versions, implement an alternative mechanism.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server_module/command_waiting_thread.py:6-6
Timestamp: 2024-10-15T12:07:47.529Z
Learning: In `external_server/server_module/command_waiting_thread.py` (Python), modifying `sys.path` at the module level is necessary to include the module `lib/fleet-protocol/protobuf/compiled/python`, which is nested deep inside a non-module folder structure.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:158-182
Timestamp: 2024-10-14T08:13:26.735Z
Learning: In the `CarServer` class, retries for connecting to the MQTT broker are implemented, so additional retry logic in `MQTTClientAdapter.connect` is unnecessary.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:76-78
Timestamp: 2024-10-14T06:49:18.563Z
Learning: In the external server Python codebase, configuration validation is module-specific and must be handled by the module implementation itself, not by adapter classes like `MQTTClientAdapter` or `APIClientAdapter`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server.py:0-0
Timestamp: 2024-10-15T07:55:43.454Z
Learning: In the `CarServer` class in `external_server/server.py`, if the connect message does not arrive in time, the connect sequence is repeated via other `CarServer` methods.
🪛 Ruff (0.11.9)
tests/libs/sniff_mqtt.py
84-84: Multiple statements on one line (colon)
(E701)
100-100: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
107-107: Multiple statements on one line (colon)
(E701)
114-114: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
121-121: Multiple statements on one line (colon)
(E701)
🔇 Additional comments (7)
tests/libs/sniff_mqtt.py (7)
1-11: LGTM! Clean import structure.The imports are well organized and include all necessary dependencies for MQTT monitoring and protobuf message handling.
18-25: LGTM! Helper function provides proper device configuration.The
mission_module_device()function correctly creates a device with appropriate module number, device type, and role for mission module testing.
28-42: LGTM! Comprehensive initialization with proper thread safety.The initialization properly sets up all required attributes including thread synchronization primitives (locks and conditions) for safe concurrent access to statuses and commands.
134-140: LGTM! Proper thread-safe status storage implementation.The status handling correctly uses condition variables for thread synchronization and implements proper message buffer management with the MAX_MESSAGES limit.
143-151: LGTM! Well-designed blocking API with proper synchronization.The
get_mission_module_statuses()method correctly implements a blocking wait pattern using condition variables, which is appropriate for test scenarios.
165-171: LGTM! Consistent command storage implementation.The command handling follows the same thread-safe pattern as status handling, ensuring consistency across the codebase.
190-212: LGTM! Comprehensive command sending implementation.The method properly validates input type, builds the complete protobuf command structure, and publishes to the correct MQTT topic with appropriate QoS settings.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tests/libs/sniff_mqtt.py (1)
44-54: Past review comment is outdatedThe syntax error mentioned in the past review comment appears to be already fixed. The
Clientconstructor call on line 45 has proper spacing.
🧹 Nitpick comments (5)
tests/libs/sniff_mqtt.py (5)
56-66: Consider optimizing client recreation in retry logicThe current implementation recreates the MQTT client on each connection retry, which might be inefficient. However, this could be intentional for ensuring a clean state after connection failures.
If the client recreation is unnecessary, consider this optimization:
def connect(self, host: str = '127.0.0.1', port: int = 1883, keepalive: int = 5) -> None: """Connects to the MQTT broker with retry logic.""" connected = False while not connected: try: - self._create_client() self._client.connect(host=host, port=port, keepalive=keepalive) connected = True except ConnectionRefusedError: time.sleep(2)
84-84: Fix formatting: Multiple statements on one lineSeparate the conditional statement and method call onto different lines for better readability.
- if self._verbose: self._print(f'topic: {message.topic}') + if self._verbose: + self._print(f'topic: {message.topic}')
107-107: Fix formatting: Multiple statements on one lineSeparate the conditional statement and method call onto different lines for better readability.
- if self._verbose: self._print(message) + if self._verbose: + self._print(message)
121-121: Fix formatting: Multiple statements on one lineSeparate the conditional statement and method call onto different lines for better readability.
- if self._verbose: self._print(message) + if self._verbose: + self._print(message)
207-212: Consider making MQTT topic configurableThe topic
'bringauto/virtual_vehicle/external_server'is hardcoded, which reduces flexibility for different test scenarios.Consider adding a configurable topic parameter:
- def send_mission_module_command(self, payload: MissionModule_pb2.AutonomyCommand) -> None: + def send_mission_module_command(self, payload: MissionModule_pb2.AutonomyCommand, topic: str = 'bringauto/virtual_vehicle/external_server') -> None: """Sends a mission module command to the MQTT broker.""" if not isinstance(payload, MissionModule_pb2.AutonomyCommand): raise TypeError('Command must be of type AutonomyCommand') # ... existing code ... self._client.publish( - topic='bringauto/virtual_vehicle/external_server', + topic=topic, payload=msg.SerializeToString(), qos=1 )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/libs/sniff_mqtt.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/libs/sniff_mqtt.py (8)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/mqtt/test_adapter.py:39-41
Timestamp: 2024-10-17T13:07:59.736Z
Learning: In `tests/mqtt/test_adapter.py` and other test files, prefer using fixed values for test configurations like `TEST_ADDRESS` and `TEST_PORT` instead of environment variables, to avoid requiring users to configure the environment for testing.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:124-124
Timestamp: 2024-10-03T05:17:30.481Z
Learning: When using the `paho-mqtt` client, accessing the private attribute `_thread` of the MQTT client directly is acceptable if it provides a more reliable way to check if the loop is running, rather than maintaining a separate flag. If `_thread` is deprecated in future versions, implement an alternative mechanism.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:124-124
Timestamp: 2024-10-09T05:03:50.249Z
Learning: When using the `paho-mqtt` client, accessing the private attribute `_thread` of the MQTT client directly is acceptable if it provides a more reliable way to check if the loop is running, rather than maintaining a separate flag. If `_thread` is deprecated in future versions, implement an alternative mechanism.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/server/mutilple_cars/test_multiple_cars.py:1-17
Timestamp: 2024-10-17T10:35:46.745Z
Learning: In `tests/server/multiple_cars/test_multiple_cars.py`, modifying `sys.path` is necessary for importing modules from the non-module directory `'lib/fleet-protocol/protobuf/compiled/python'`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server_module/command_waiting_thread.py:6-6
Timestamp: 2024-10-15T12:07:47.529Z
Learning: In `external_server/server_module/command_waiting_thread.py` (Python), modifying `sys.path` at the module level is necessary to include the module `lib/fleet-protocol/protobuf/compiled/python`, which is nested deep inside a non-module folder structure.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/api/adapter.py:76-78
Timestamp: 2024-10-14T06:49:18.563Z
Learning: In the external server Python codebase, configuration validation is module-specific and must be handled by the module implementation itself, not by adapter classes like `MQTTClientAdapter` or `APIClientAdapter`.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/adapters/mqtt/adapter.py:158-182
Timestamp: 2024-10-14T08:13:26.735Z
Learning: In the `CarServer` class, retries for connecting to the MQTT broker are implemented, so additional retry logic in `MQTTClientAdapter.connect` is unnecessary.
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: external_server/server.py:0-0
Timestamp: 2024-10-15T07:55:43.454Z
Learning: In the `CarServer` class in `external_server/server.py`, if the connect message does not arrive in time, the connect sequence is repeated via other `CarServer` methods.
🪛 Ruff (0.11.9)
tests/libs/sniff_mqtt.py
84-84: Multiple statements on one line (colon)
(E701)
100-100: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
107-107: Multiple statements on one line (colon)
(E701)
114-114: Do not call getattr with a constant attribute value. It is not any safer than normal property access.
Replace getattr with attribute access
(B009)
121-121: Multiple statements on one line (colon)
(E701)
🔇 Additional comments (4)
tests/libs/sniff_mqtt.py (4)
1-11: LGTM: Clean imports structureThe import organization is clear and follows proper conventions. Using aliased imports for protobuf modules improves readability.
143-151: LGTM: Thread-safe message retrieval with proper blockingThe implementation correctly uses condition variables for thread-safe access and provides blocking wait functionality until new messages arrive. The pattern of copying and clearing the list is appropriate for the use case.
192-194: LGTM: Proper type checking for command parameterThe type validation ensures that only valid
AutonomyCommandprotobuf messages are processed, which prevents runtime errors.
215-219: LGTM: Simple and effective test executionThe main block provides a straightforward way to run the monitoring utility standalone, which is useful for debugging and manual testing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/README.md (3)
14-14: Correct typo “vertion” → “version”.Small spelling fix keeps the instructions professional and avoids confusion when grepping for keywords like version.
- - Change the mission module vertion in the module gateway Dockerfile to the new branch name + - Change the mission module version in the module gateway Dockerfile to the new branch name
17-22: Usepython -m pip& upgrade pip for better venv portability.Calling
python -m pipensures the venv’s interpreter is used (works on systems wherepip3isn’t on $PATH) and upgrading pip avoids install-time surprises on older system images.cd tests python3 -m venv .venv source .venv/bin/activate -pip3 install -r requirements.txt +python -m pip install --upgrade pip +python -m pip install -r requirements.txt
24-37: Consistently capitalise “MQTT” & tighten command section.The doc jumps between
mqtt/MQTT; using the standard all-caps acronym is clearer. While you’re editing, consider adding the optional flags (--logging,--verbose) to the example command so newcomers spot them immediately.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/README.md(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
tests/README.md (5)
Learnt from: jiristrouhal
PR: bringauto/external-server#26
File: tests/mqtt/test_adapter.py:39-41
Timestamp: 2024-10-17T13:07:59.736Z
Learning: In tests/mqtt/test_adapter.py and other test files, prefer using fixed values for test configurations like TEST_ADDRESS and TEST_PORT instead of environment variables, to avoid requiring users to configure the environment for testing.
Learnt from: mikusaq
PR: bacpack-system/packager#24
File: README.md:0-0
Timestamp: 2024-09-30T11:48:14.934Z
Learning: The TODO section in README.md was outdated and has been removed.
Learnt from: mikusaq
PR: bacpack-system/packager#24
File: README.md:0-0
Timestamp: 2024-10-09T05:03:50.249Z
Learning: The TODO section in README.md was outdated and has been removed.
Learnt from: jiristrouhal
PR: bringauto/fleet-management-http-api#27
File: tests/controllers/car/test_get_last_car_state.py:25-25
Timestamp: 2025-01-31T13:35:03.741Z
Learning: The fleet-management-http-api codebase includes tenant isolation tests across multiple test files, making additional car-specific tenant isolation tests in test_get_last_car_state.py redundant.
Learnt from: MarioIvancik
PR: #19
File: CMakeLists.txt:13-13
Timestamp: 2024-10-23T09:33:50.060Z
Learning: In the mission module project, due to bugs in the protobuf library, messages are always sent as raw JSON, and protobuf is used solely for data validation.
🪛 LanguageTool
tests/README.md
[grammar] ~14-~14: Ensure spelling is correct
Context: ... new branch - Change the mission module vertion in the module gateway Dockerfile to the...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
|
https://youtrack.bringauto.com/issue/BAF-1107/Mission-module-error-aggregation-in-stop
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
.gitignorefiles to exclude virtual environments and build artifacts.