Skip to content

Key encoding unit test flakes on wall-clock performance bound #6745

Closed

Description

@larrysingleton007

Expected Behavior

The parallel unit suite should not fail because a shared CI runner briefly provides less CPU time to one pytest worker. Key encoding performance should be tracked by the benchmark suite.

Current Behavior

test_performance_bounds_single_entity measures 1,000 serialization and deserialization calls with time.perf_counter() and requires each loop to finish within 0.2 seconds. The unit suite runs with 8 pytest-xdist workers, so elapsed time includes time that the worker is descheduled.

The test failed on master in run 31768603920:

AssertionError: Serialization too slow: 0.2062s
assert 0.20623705900004552 < 0.2

This limit has already been raised twice to address CI failures. It started at 0.02 seconds in #5981, changed to 0.1 seconds in #6040, then changed to 0.2 seconds in #6043. Raising it again would postpone the next failure without making the measurement deterministic.

Steps to reproduce

  1. Run make test-python-unit, which starts 8 pytest workers.
  2. Let test_performance_bounds_single_entity execute while the runner is under contention.
  3. The wall-clock loop can exceed 0.2 seconds even though the implementation has not regressed.

The intermittent failure is captured in the linked master run. Local execution passes when the host is idle, which is consistent with runner contention.

Specifications

  • Version: current master at f771ea4
  • Failing platform: Python 3.10 on ubuntu-latest
  • Subsystem: Python unit tests, entity key encoding
  • File: sdk/python/tests/unit/infra/test_key_encoding_utils.py

Possible Solution

Remove test_performance_bounds_single_entity. The same change that introduced it also added dedicated pytest-benchmark coverage in sdk/python/tests/benchmarks/test_key_encoding_benchmarks.py for single-entity string and integer serialization and deserialization. Those benchmarks run in the separate benchmark workflow and preserve the performance signal without turning shared-runner latency into a unit-test failure.

The existing test_single_entity_fast_path continues to cover correctness of the optimized path.

Activity

  1. larrysingleton007 commented on Aug 16, 2026

    @larrysingleton007
    ContributorAuthor

    Implementation Plan

    Problem Summary

    test_performance_bounds_single_entity treats shared-runner wall-clock latency as a unit-test failure, despite the same paths having dedicated benchmark coverage.

    Proposed Solution

    Remove the redundant wall-clock unit test. Keep test_single_entity_fast_path for correctness and retain the dedicated single-entity serialization and deserialization benchmarks for performance tracking.

    Files to Modify

    • sdk/python/tests/unit/infra/test_key_encoding_utils.py

    New Files

    • None

    Testing Approach

    • Run the affected unit-test module.
    • Run the dedicated single-entity serialization and deserialization benchmarks.
    • Run formatting, lint, pre-commit, and the relevant broader Python unit checks.
  2. larrysingleton007 commented on Aug 16, 2026

    @larrysingleton007
    ContributorAuthor

    Implemented in #6746.

    The PR removes the absolute wall-clock assertion from the parallel unit suite while retaining correctness coverage and the dedicated pytest-benchmark measurements. The complete Python unit suite passes locally with 2,536 passed and 20 skipped.

  3. added 2 commits that reference this issue on Aug 18, 2026
    bf74d29
    aa7b685
  4. added a commit that references this issue on Aug 31, 2026
    9342f4b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions