Skip to content

fix(sdk/metrics): make test_collect_resets_start_time_unix_nano deterministic on PyPy/Windows#5154

Draft
Copilot wants to merge 2 commits intomainfrom
copilot/fix-pypy-windows-ci-test
Draft

fix(sdk/metrics): make test_collect_resets_start_time_unix_nano deterministic on PyPy/Windows#5154
Copilot wants to merge 2 commits intomainfrom
copilot/fix-pypy-windows-ci-test

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 27, 2026

Description

test_collect_resets_start_time_unix_nano was flaky on PyPy/Windows because it patched opentelemetry.sdk.metrics._internal._view_instrument_match.time_ns with side_effect=[0, 1, 2] while simultaneously calling the real system time_ns() via the locally-imported from time import time_ns. The mocked clock (returning 0, 1, 2) and the real wall clock were mixed into the same assertions, causing nondeterministic failures where clock resolution on Windows/PyPy is coarser.

Fix: Replace the 7 real time_ns() calls inside test_collect_resets_start_time_unix_nano with fixed literals — the test is verifying _ViewInstrumentMatch internal start-time reset behavior, not anything about wall-clock values.

  • time_unix_nano=time_ns() in Measurement constructors → fixed literals (123, 789, 1000, 1001, 1002)
  • collection_start_time_unix_nano = time_ns() → inlined as 456 and 999 at the collect() call sites

The from time import time_ns import is kept; it is used by other tests in the file.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • test_collect_resets_start_time_unix_nano — passes deterministically
  • Full test_view_instrument_match.py suite — all 9 tests pass

Does This PR Require a Contrib Repo Change?

  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated
Original prompt

Fix the flaky/failing PyPy Windows CI test in repository open-telemetry/opentelemetry-python.

The failing job is from GitHub Actions run 24913368190, job 72959997694, on commit 715ed3a253c778d12931009a8e9e038d57892289.

Observed failure:

  • Test: opentelemetry-sdk/tests/metrics/test_view_instrument_match.py::Test_ViewInstrumentMatch::test_collect_resets_start_time_unix_nano
  • In the logs for the failing job reference 715ed3a253c778d12931009a8e9e038d57892289, this is the only failed test.

Root cause to address:

  • The test patches opentelemetry.sdk.metrics._internal._view_instrument_match.time_ns but also uses the locally imported time_ns from from time import time_ns inside the test body.
  • This mixes mocked and real clock values, making the test nondeterministic, especially on Windows/PyPy.
  • Update the test so it uses deterministic values consistently and does not rely on the real wall clock in this mocked-time scenario.

Files to inspect and modify as needed:

  • opentelemetry-sdk/tests/metrics/test_view_instrument_match.py
  • If needed for context only: opentelemetry-sdk/src/opentelemetry/sdk/metrics/_internal/export/__init__.py
  • Workflow context: .github/workflows/ci.yml

Expected outcome:

  • Make test_collect_resets_start_time_unix_nano deterministic across environments.
  • Keep the behavior being asserted the same: verify that _ViewInstrumentMatch resets the start time for new aggregations after collection.
  • Prefer the smallest focused test-only change.

Please create a pull request with the fix and a concise description of the root cause and resolution.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Please find a solution for this failing job. Use the logs, job definition, and any referenced files where the failure occurred. Keep your response focused on the solution and include code suggestions when appropriate.

Assistant: The failing test is opentelemetry-sdk/tests/metrics/test_view_instrument_match.py::Test_ViewInstrumentMatch::test_collect_resets_start_time_unix_nano in the PyPy Windows job.

Root cause: the test assumes time_ns() always advances as 0, 1, 2, but the test itself mixes two different time_ns functions:

Those local calls happen at:

That makes the test nondeterministic, especially on Windows/PyPy.

Fix

Make the test use the mocked clock consistently. Replace the direct time_ns() calls with explicit constants or values derived from the patched function argument.

A minimal, stable rewrite is:

@patch(
    "opentelemetry.sdk.metrics._internal._view_instrument_match.time_ns",
    side_effect=[0, 1, 2],
)
def test_collect_resets_start_time_unix_nano(self, mock_time_ns):
    instrument = Mock(name="instrument")
    instrument.instrumentation_scope = self.mock_instrumentation_scope
    view_instrument_match = _ViewInstrumentMatch(
        view=View(
            instrument_name="instrument",
            name="name",
            aggregation=self.mock_aggregation_factory,
        ),
     ...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

…yPy/Windows

Replace real time_ns() calls inside the test body with fixed literal
values. The test patches the module-level time_ns but was still calling
the real time_ns() from `from time import time_ns`, mixing mocked and
real clock values which caused nondeterminism on Windows/PyPy.

Assisted-by: Claude Opus 4.6

Agent-Logs-Url: https://github.com/open-telemetry/opentelemetry-python/sessions/90d2ba7b-9d8d-428d-bc85-e0631ee832ec

Co-authored-by: xrmx <12932+xrmx@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix flaky PyPy Windows CI test in metrics view instrument fix(sdk/metrics): make test_collect_resets_start_time_unix_nano deterministic on PyPy/Windows Apr 27, 2026
Copilot finished work on behalf of xrmx April 27, 2026 09:36
Copilot AI requested a review from xrmx April 27, 2026 09:36
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.

2 participants