|WARNING| [v3] dts: update dts check format script and resolve errors

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Fri Jun 26 02:56:46 CEST 2026


Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/166275

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-26

# Review of DPDK Patch: dts-check-format.sh tool version update

## Summary

This patch updates tool versions in the DTS check-format script and resolves formatting/type-hinting errors. Overall, the patch is well-structured and addresses legitimate code quality issues. However, there are several type hinting issues that need correction.

---

## Errors

### 1. Incorrect return type annotations in decorator wrappers

**File**: `dts/api/testpmd/__init__.py`

**Lines 79-82, 105-108, 135-138**

The return type annotations for decorator wrapper functions are incorrect. These wrappers should return the result type `R` of the wrapped function, not `Callable[...]`.

```python
# Current (incorrect):
def _wrapper(
    self: "TestPmd", *args: P.args, **kwargs: P.kwargs
) -> Callable[Concatenate["TestPmd", P], Any]:
    # ...

# Should be:
def _wrapper(
    self: "TestPmd", *args: P.args, **kwargs: P.kwargs
) -> R:
    # ...
```

All three decorators (`_requires_stopped_ports`, `_requires_started_ports`, and the wrapper inside `_add_remove_mtu`) have this same issue. The wrapper functions execute and return the result of the wrapped function, so they should be annotated with return type `R`, not with a `Callable` type.

### 2. Incorrect return type in interactive shell decorator

**File**: `dts/framework/remote_session/interactive_shell.py`

**Lines 52-55**

Similar issue to above - the return type should be `R | None`, not `Callable[P, "InteractiveShell"] | None`.

```python
# Current (incorrect):
def _wrapper(
    self: "InteractiveShell", *args: P.args, **kwargs: P.kwargs
) -> Callable[P, "InteractiveShell"] | None:
    # ...

# Should be:
def _wrapper(
    self: "InteractiveShell", *args: P.args, **kwargs: P.kwargs
) -> R | None:
    # ...
```

### 3. Incorrect return type in context.py decorator

**File**: `dts/framework/context.py`

**Lines 111**

The inner decorator should return `Callable[P, Any]`, not `Callable[P, type["TestProtocol"]]`, since it wraps a test method (which is a function), not a class.

```python
# Current (incorrect):
def decorator(func: type["TestProtocol"]) -> Callable[P, type["TestProtocol"]]:
    # ...

# Should be:
def decorator(func: type["TestProtocol"]) -> Callable[P, Any]:
    # ...
```

Actually, on closer inspection, the parameter type `func: type["TestProtocol"]` is also wrong - it should be `Callable[P, Any]` or similar, since `func` is a function being decorated, not a class. The original code signature and intent here is unclear, but the current types are definitely incorrect.

---

## Warnings

### 1. Poetry lock file version change

**File**: `dts/poetry.lock`

The Poetry lock file shows a metadata change from `lock-version = "2.0"` to `lock-version = "2.1"` and notes it was generated with Poetry 2.3.2 instead of 1.8.3. This is expected with the tool version update, but should be verified that all developers/CI have compatible Poetry versions to avoid lock file churn.

### 2. Significant dependency version jumps

**Files**: `dts/poetry.lock`, `dts/pyproject.toml`

Several major version updates:
- `mypy` from `^1.13.0` to `^2.1.0` (major version bump)
- `ruff` from `^0.8.1` to `^0.15.16` (significant version jump)
- `types-paramiko` from `^3.5.0` to `^4.0.0` (major version bump)

While these updates are intentional per the commit message, the major version bumps (especially mypy 2.x) may introduce breaking changes in type checking behavior. Ensure CI passes and no new false positives are introduced.

---

## Info

### 1. Code formatting improvements are correct

The string concatenation cleanups, f-string formatting fixes, and line-breaking improvements throughout the patch are all correct and improve readability. Examples:
- Joining multi-line strings with `\n` embedded in f-strings (e.g., `dts/framework/remote_session/remote_session.py`)
- Fixing spacing around operators (e.g., `retry_attempt + 1` instead of `retry_attempt +1`)
- Consistent quote usage in f-strings

### 2. Type annotation improvements

The explicit type annotations added for flag initialization (e.g., `flag: RSSOffloadTypesFlag = cls(0)` in `dts/api/testpmd/types.py`) are good practice and help the type checker understand the intended types.

### 3. Tarball mode type safety

**File**: `dts/framework/utils.py`

The addition of `Tar_modes` TypeAlias and the explicit `cast` when opening tarballs is a good defensive practice for ensuring type safety with the `tarfile` module's mode strings.

---

## Conclusion

The patch successfully updates tool versions and addresses most formatting issues. However, **the return type annotations in several decorators are incorrect and must be fixed** (Errors section). These are type-checking correctness issues that should be caught by the updated mypy 2.x.

The formatting changes themselves are all appropriate and improve code quality.


More information about the test-report mailing list