Skip to content

test(e2e): fix mypy Optional narrowing in test_session_runtime #1046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
epmog merged 2 commits into from
Aug 11, 2026
+23 −0
Merged

test(e2e): fix mypy Optional narrowing in test_session_runtime #1046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4abed77
test(e2e): fix mypy Optional narrowing in test_session_runtime
seant-aws Aug 11, 2026
0bc8bba
ci: run e2e:lint in Code Quality workflow
seant-aws Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .github/workflows/code_quality.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,25 @@ jobs:

- name: Run Integration Tests
run: hatch run integ-test

E2ELint:
name: E2E Lint
runs-on: ubuntu-latest
permissions:
contents: read
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
ref: ${{ inputs.tag || inputs.branch || github.ref }}

- name: Set up Python
uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
with:
python-version: '3.11'

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinning this gate to a single interpreter means the e2e type check only ever runs one of the two mypy tracks the repo supports. requirements-e2e.txt splits on interpreter version:

mypy >= 2.3.0, < 2.4; python_version >= "3.10"
mypy >= 1.19.1, < 1.20; python_version < "3.10"

so CI will only exercise mypy 2.3 under 3.11, while a contributor on 3.9 gets mypy 1.19 locally. Separately, test/e2e/mypy.ini sets no python_version, so mypy infers 3.11 from the running interpreter — version-conditional typing errors under the project minimum (requires-python = ">=3.9") will not be caught here.

Two options depending on intent: either add python_version = 3.9 to test/e2e/mypy.ini so the check is pinned to the floor regardless of which interpreter CI happens to use, or give this job the same python-version matrix as UnitTest/IntegrationTests above so both mypy tracks are covered. The former is cheaper and probably sufficient; the ruff half of e2e:lint is version-independent either way.


- name: Install Hatch
run: |
pip install --upgrade hatch

- name: Run E2E Lint
run: hatch run e2e:lint
1 change: 1 addition & 0 deletions test/e2e/test_session_runtime.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@ def check_worker_service_stopped() -> None:
interval=10,
)
def wait_worker_started() -> None:
assert worker.worker_id is not None # narrow Optional inside the closure for mypy

@github-actions github-actions Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This narrowing assert now lives inside a closure decorated with @backoff.on_exception(backoff.constant, Exception, max_time=120, interval=10), so it is caught and retried like a transient failure. If worker.worker_id were ever None, this would burn the full 120s retry window and then surface as a bare AssertionError with no message, rather than failing fast with a clear cause.

Note also that line 531 already does assert worker.worker_id is not None immediately above — mypy just does not carry narrowing of a member expression into a nested function body, which is why the duplicate is needed here.

A cleaner fix that satisfies mypy without putting a non-retryable precondition inside the retry loop is to bind a local before the closure:

worker_id = worker.worker_id
assert worker_id is not None

@backoff.on_exception(backoff.constant, Exception, max_time=120, interval=10)
def wait_worker_started() -> None:
    assert is_worker_started(
        deadline_client=deadline_client,
        farm_id=deadline_resources.farm.id,
        fleet_id=deadline_resources.fleet.id,
        worker_id=worker_id,
    )

A local variable stays narrowed inside the closure, so the assert is checked once, up front, and the duplication on line 531 goes away.

assert is_worker_started(
deadline_client=deadline_client,
farm_id=deadline_resources.farm.id,
Expand Down
Loading