Skip to content

Make flaky connection-type tests more lenient#1643

Open
rolandwalker wants to merge 1 commit intomainfrom
RW/more-lenient-flaky-connection-test
Open

Make flaky connection-type tests more lenient#1643
rolandwalker wants to merge 1 commit intomainfrom
RW/more-lenient-flaky-connection-test

Conversation

@rolandwalker
Copy link
Contributor

Description

Depending on the situation, an intended UDS connection can fall back to a TCP/IP connection, so the status == UDS test can fail.

Assuming the inverse is true, make TCP/IP and UDS status-text checks each depend on whether the unix_socket property is actually set on the connection. This evades one purpose of the tests, but still tests the text in the status output.

An alternative could be to delete the flaky tests entirely.

These tests are a problem in my local, but not failing in CI.

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

Depending on the situation, an intended UDS connection can fall back to
a TCP/IP connection, so the status == UDS test can fail.

Assuming the inverse is true, make TCP/IP and UDS status-text checks
each depend on whether the "unix_socket" property is actually set on the
connection.  This evades one purpose of the tests, but still tests the
text in the status output.

An alternative could be to delete the flaky tests entirely.
@rolandwalker rolandwalker self-assigned this Feb 27, 2026
@github-actions
Copy link

Findings

  1. High: assertion source is unrelated to the connection under test

    • The new branching logic uses context.cn.unix_socket in [connection.py:27] and [connection.py:45], but context.cn is a separate setup connection (created in before_all) rather than the mycli session being validated.
    • This can make Then status consistent with socket [connection.feature:7] / [connection.feature:23] assert "via TCP/IP" and still pass, masking real regressions in status.
    • Action: Base expectation on the actual mycli connection output/inputs for that scenario (or make the scenario deterministic), not on context.cn.
  2. Medium: PR removes coverage of intended behavior

    • Replacing strict checks with “either socket or TCP/IP” means these scenarios no longer verify the host/port-to-transport mapping they were originally testing.
    • Action: Keep these integration tests deterministic (or skip flaky envs), and add focused tests for status rendering so both "via UNIX socket" and "via TCP/IP" are each explicitly exercised.

No direct security issues found in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant