Skip to content

Commit

Permalink
chore(api): apply mypy consistently across api
Browse files Browse the repository at this point in the history
At present, typechecking is not applied fully and consistently across the API directory. This PR
removes exceptions to mypy checking rules and makes sure that all mypy typechecking tests pass.

EXEC-201 EXEC-202
  • Loading branch information
aaron-kulkarni committed Jul 29, 2024
1 parent 90db6a7 commit d20f46d
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 35 deletions.
6 changes: 3 additions & 3 deletions api/mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ warn_return_any = False

# ~30 errors
[mypy-tests.opentrons.drivers.*]
disallow_untyped_defs = False
disallow_untyped_calls = False
disallow_incomplete_defs = False
# disallow_untyped_defs = False
# disallow_untyped_calls = False
# disallow_incomplete_defs = False

[mypy-tests.opentrons.protocol_api_old.*]
disallow_untyped_defs = False
Expand Down
4 changes: 3 additions & 1 deletion api/tests/opentrons/drivers/absorbance_reader/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ class MockErrorCode(Enum):


@pytest.fixture
async def mock_async_byonoy(mock_interface, mock_device) -> AsyncByonoy:
async def mock_async_byonoy(
mock_interface: MagicMock, mock_device: MagicMock
) -> AsyncByonoy:
loop = asyncio.get_running_loop()
return AsyncByonoy(
mock_interface, mock_device, ThreadPoolExecutor(max_workers=1), loop
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ async def test_write_timeout_override(
mock_write_timeout_prop: PropertyMock,
default: Optional[int],
override: Optional[int],
):
) -> None:
"""It should override the timeout and return to default after the call."""
mock_write_timeout_prop.return_value = default
async with subject.timeout_override("write_timeout", override):
Expand All @@ -78,7 +78,7 @@ async def test_timeout_override(
mock_timeout_prop: PropertyMock,
default: Optional[int],
override: Optional[int],
):
) -> None:
"""It should override the timeout and return to default after the call."""
mock_timeout_prop.return_value = default
async with subject.timeout_override("timeout", override):
Expand All @@ -101,7 +101,7 @@ async def test_write_timeout_dont_override(
mock_write_timeout_prop: PropertyMock,
default: Optional[int],
override: Optional[int],
):
) -> None:
"""It should not override the timeout if not necessary."""
mock_write_timeout_prop.return_value = default
async with subject.timeout_override("write_timeout", override):
Expand All @@ -123,7 +123,7 @@ async def test_read_timeout_dont_override(
mock_timeout_prop: PropertyMock,
default: Optional[int],
override: Optional[int],
):
) -> None:
"""It should not override the timeout if not necessary."""
mock_timeout_prop.return_value = default
async with subject.timeout_override("timeout", override):
Expand All @@ -132,7 +132,7 @@ async def test_read_timeout_dont_override(
mock_timeout_prop.assert_called_once()


def test_reset_input_buffer(mock_serial: MagicMock, subject: AsyncSerial):
def test_reset_input_buffer(mock_serial: MagicMock, subject: AsyncSerial) -> None:
"""It should call the underlying serial port's Reset function"""
subject.reset_input_buffer()
mock_serial.reset_input_buffer.assert_called_once()
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from typing import Type, Union

from typing import Type, Union, AsyncGenerator
import pytest
from _pytest.fixtures import SubRequest
from mock import AsyncMock, call
Expand Down Expand Up @@ -83,7 +82,9 @@ async def async_subject(


@pytest.fixture
async def subject_raise_on_error_patched(async_subject):
async def subject_raise_on_error_patched(
async_subject: AsyncResponseSerialConnection,
) -> AsyncGenerator[AsyncResponseSerialConnection, None]:
raise_on_error_mock = mock.MagicMock()
with mock.patch.object(async_subject, "raise_on_error", raise_on_error_mock):
yield async_subject
Expand Down
7 changes: 4 additions & 3 deletions api/tests/opentrons/drivers/rpi_drivers/test_usb.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from _pytest.fixtures import SubRequest
import pytest
from mock import patch, MagicMock
from typing import Iterator, cast
Expand Down Expand Up @@ -58,14 +59,14 @@


@pytest.fixture(params=[BoardRevision.OG, BoardRevision.A, BoardRevision.FLEX_B2])
def revision(request) -> BoardRevision:
def revision(request: SubRequest) -> BoardRevision:
return cast(BoardRevision, request.param)


@pytest.fixture()
def usb_bus(revision: BoardRevision) -> Iterator[USBBus]:
@staticmethod # type: ignore[misc]
def fake_read_bus():
def fake_read_bus() -> list[str]:
if revision == BoardRevision.OG:
return fake_bus_og
elif revision == BoardRevision.A:
Expand All @@ -80,7 +81,7 @@ def fake_read_bus():
yield USBBus(revision)


def test_modify_module_list(revision: BoardRevision, usb_bus: USBBus):
def test_modify_module_list(revision: BoardRevision, usb_bus: USBBus) -> None:
# TODO(mc, 2022-03-01): partial patching the class under test creates
# a contaminated test subject that reduces the value of these tests
# https://github.com/testdouble/contributing-tests/wiki/Partial-Mock
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ async def test_command_sender_sanitized_response(
],
],
)
def test_remove_serial_echo(cmd: str, resp: str, expected: str):
def test_remove_serial_echo(cmd: str, resp: str, expected: str) -> None:
"""It should remove unwanted characters only."""
res = SmoothieConnection._remove_unwanted_characters(cmd, resp)
assert res == expected
26 changes: 16 additions & 10 deletions api/tests/opentrons/drivers/smoothie_drivers/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from mock import AsyncMock, patch
import pytest
from opentrons.drivers.smoothie_drivers.connection import SmoothieConnection
from opentrons.drivers.rpi_drivers.dev_types import GPIODriverLike
from opentrons.drivers.smoothie_drivers.constants import (
HOMED_POSITION,
Y_BOUND_OVERRIDE,
Expand All @@ -14,6 +15,7 @@

from opentrons.drivers.smoothie_drivers import driver_3_0, constants
from opentrons.drivers.smoothie_drivers.errors import SmoothieError, SmoothieAlarm
from opentrons.drivers.command_builder import CommandBuilder


@pytest.fixture
Expand All @@ -29,7 +31,9 @@ def sim_gpio() -> SimulatingGPIOCharDev:


@pytest.fixture
def smoothie(mock_connection: AsyncMock, sim_gpio) -> driver_3_0.SmoothieDriver:
def smoothie(
mock_connection: AsyncMock, sim_gpio: GPIODriverLike
) -> driver_3_0.SmoothieDriver:
"""The smoothie driver under test."""
from opentrons.config import robot_configs

Expand All @@ -41,7 +45,9 @@ def smoothie(mock_connection: AsyncMock, sim_gpio) -> driver_3_0.SmoothieDriver:
return d


def position(x, y, z, a, b, c):
def position(
x: float, y: float, z: float, a: float, b: float, c: float
) -> Dict[str, float]:
return {axis: value for axis, value in zip("XYZABC", [x, y, z, a, b, c])}


Expand Down Expand Up @@ -76,7 +82,7 @@ async def test_update_position_retry(
assert smoothie.position == expected


def test_active_dwelling_current_push_pop(smoothie):
def test_active_dwelling_current_push_pop(smoothie: driver_3_0.SmoothieDriver) -> None:
assert smoothie._active_current_settings != smoothie._dwelling_current_settings

old_active_currents = deepcopy(smoothie._active_current_settings)
Expand All @@ -90,7 +96,7 @@ def test_active_dwelling_current_push_pop(smoothie):
assert smoothie._dwelling_current_settings == old_dwelling_currents


async def test_functional(smoothie: driver_3_0.SmoothieDriver):
async def test_functional(smoothie: driver_3_0.SmoothieDriver) -> None:
smoothie.simulating = True
assert smoothie.position == position(0, 0, 0, 0, 0, 0)

Expand All @@ -116,7 +122,7 @@ async def test_functional(smoothie: driver_3_0.SmoothieDriver):

async def test_read_pipette_v13(
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
):
) -> None:
mock_connection.send_command.return_value = "L:" + utils.string_to_hex(
"p300_single_v13"
)
Expand All @@ -126,7 +132,7 @@ async def test_read_pipette_v13(

async def test_switch_state(
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
):
) -> None:
smoothie_switch_res = (
"X_max:0 Y_max:0 Z_max:0 A_max:0 B_max:0 C_max:0"
" _pins "
Expand Down Expand Up @@ -172,7 +178,7 @@ async def test_switch_state(

async def test_clear_limit_switch(
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
):
) -> None:
"""
This functions as a contract test around recovery from a limit-switch hit.
Note that this *does not* itself guarantee correct physical behavior--this
Expand All @@ -182,7 +188,7 @@ async def test_clear_limit_switch(
"""
cmd_list = []

async def write_mock(command, retries, timeout):
async def write_mock(command: CommandBuilder, retries: int, timeout: float) -> str:
cmd_list.append(command.build())
if constants.GCODE.MOVE in command:
raise AlarmResponse(port="", response="ALARM: Hard limit +C")
Expand Down Expand Up @@ -223,10 +229,10 @@ async def write_mock(command, retries, timeout):

async def test_unstick_axes(
smoothie: driver_3_0.SmoothieDriver, mock_connection: AsyncMock
):
) -> None:
cmd_list = []

def write_mock(command, retries, timeout):
def write_mock(command: CommandBuilder, retries: int, timeout: float) -> str:
cmd_list.append(command.build())
if constants.GCODE.LIMIT_SWITCH_STATUS in command:
return "X_max:0 Y_max:0 Z_max:0 A_max:0 B_max:0 C_max:0 Probe: 0 \r\n"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from opentrons.drivers.utils import ParseError


def test_parse_position_response():
def test_parse_position_response() -> None:
good_data = "ok M114.2 X:10 Y:20 Z:30 A:40 B:50 C:60"
res = parse_utils.parse_position_response(good_data)
expected = {
Expand All @@ -26,12 +26,12 @@ def test_parse_position_response():
["ok M114.2 X:10 Y:20: Z:30 A:40 B:50"],
],
)
def test_parse_position_response_error(data: str):
def test_parse_position_response_error(data: str) -> None:
with pytest.raises(ParseError):
parse_utils.parse_position_response(data)


def test_parse_pipette_data():
def test_parse_pipette_data() -> None:
msg = "TestsRule!!"
mount = "L"
good_data = mount + ": " + utils.string_to_hex(msg)
Expand All @@ -49,6 +49,6 @@ def test_parse_pipette_data():
["L:0.212"],
],
)
def test_parse_pipette_data_error(data: str):
def test_parse_pipette_data_error(data: str) -> None:
with pytest.raises(ParseError):
parse_utils.parse_instrument_data(data)
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import pytest
from typing import Dict
from opentrons.drivers.smoothie_drivers import SmoothieDriver

from opentrons.drivers.smoothie_drivers.constants import AXES, HOMED_POSITION as HP

POSITION_ON_BOOT = {axis: 0 for axis in AXES}


def test_driver_init(smoothie: SmoothieDriver):
def test_driver_init(smoothie: SmoothieDriver) -> None:
assert smoothie.position == POSITION_ON_BOOT


async def test_home(smoothie: SmoothieDriver):
async def test_home(smoothie: SmoothieDriver) -> None:
await smoothie.home()
assert smoothie.position == HP

Expand Down Expand Up @@ -39,7 +40,11 @@ async def test_home(smoothie: SmoothieDriver):
),
],
)
async def test_move(target_movement, expected_new_position, smoothie: SmoothieDriver):
async def test_move(
target_movement: Dict[str, float],
expected_new_position: Dict[str, float],
smoothie: SmoothieDriver,
) -> None:
await smoothie.home()
await smoothie.move(target_movement)
assert smoothie.position == expected_new_position
4 changes: 2 additions & 2 deletions api/tests/opentrons/drivers/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def test_parse_rpm_response_failure(input_str: str) -> None:
)
def test_parse_labware_latch_status_response_success(
input_str: str, expected_result: HeaterShakerLabwareLatchStatus
):
) -> None:
assert utils.parse_labware_latch_status_response(input_str) == expected_result


Expand All @@ -179,7 +179,7 @@ def test_parse_labware_latch_status_response_success(
[":"],
],
)
def test_parse_labware_latch_status_response_failure(input_str):
def test_parse_labware_latch_status_response_failure(input_str: str) -> None:
with pytest.raises(utils.ParseError):
utils.parse_labware_latch_status_response(input_str)

Expand Down

0 comments on commit d20f46d

Please sign in to comment.