Skip to content

test: xfail v1 UNI-exchange price-output cases (no liquidity) - #479

Closed
TimeToBuildBob wants to merge 1 commit into
uniswap-python:masterfrom
TimeToBuildBob:fix/v1-uni-price-xfail
Closed

TimeToBuildBob wants to merge 1 commit into
uniswap-python:masterfrom
TimeToBuildBob:fix/v1-uni-price-xfail

Conversation

@TimeToBuildBob

Copy link
Copy Markdown
Contributor

Problem

The test (v1, mainnet) job fails on every PR with 3 identical failures: the ETH-UNI / UNI-ETH test_get_price_output parametrizations hit the Uniswap v1 UNI exchange contract, which has no remaining liquidity — the Vyper bytecode raises EVM error InvalidJump. This is pre-existing (same failures on the first push of #478) and unrelated to any current change.

Fix

Runtime pytest.xfail() in test_get_price_output when version == 1 and the pair involves UNI. v2/v3 UNI cases keep passing, and any other v1 pair is unaffected. 2 lines.

Verification

Ref: #478

…dJump)

The three ETH-UNI / UNI-ETH test_get_price_output parametrizations fail on the
v1 mainnet job because the v1 UNI exchange contract has no remaining liquidity
and the Vyper bytecode hits EVM error InvalidJump. This is pre-existing
(depot unchanged since the branch's first push) and blocks every PR's CI.

Runtime-xfail only fires for version==1 UNI pairs; v2/v3 UNI cases keep passing.

Git-Session-Id: 3d27
@codecov

codecov Bot commented Sep 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.26%. Comparing base (bc8fdf4) to head (9fcbe22).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #479   +/-   ##
=======================================
  Coverage   73.26%   73.26%           
=======================================
  Files          12       12           
  Lines        2319     2319           
=======================================
  Hits         1699     1699           
  Misses        620      620           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@greptile-apps

greptile-apps Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR marks the three V1 UNI exact-output quote cases as expected failures because the liquidity-free legacy exchange reverts on the mainnet fork.

  • Limits the runtime xfail to V1 cases involving UNI.
  • Leaves V2/V3 UNI cases and all non-UNI V1 cases unchanged.

Confidence Score: 5/5

The PR appears safe to merge because the new condition covers exactly the documented failing V1 UNI output-price cases.

The xfail is restricted by both protocol version and token symbol, while every intercepted parameterization reaches the legacy UNI exchange and previously fails rather than taking the existing unsupported-pair skip.

Important Files Changed

Filename Overview
tests/test_uniswap.py Adds a narrowly scoped runtime xfail for the three documented V1 UNI output-price cases without suppressing other parameterizations.

Reviews (1): Last reviewed commit: "test: xfail v1 UNI-exchange price-output..." | Re-trigger Greptile

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

Friendly ping for review when you get a chance — all 7 checks are green on this one (including the previously-failing v1 mainnet job). Once it merges, PR #478's last red check should resolve on re-run.

TimeToBuildBob added a commit to TimeToBuildBob/uniswap-python that referenced this pull request Sep 2, 2026
Same fix as uniswap-python#479 — the v1 UNI exchange has no remaining liquidity so
get_price_output raises EVM error InvalidJump on mainnet fork.

Git-Session-Id: 8f77
@liquid-8

Copy link
Copy Markdown
Member

Imo, we can dismiss this issue completely. v1 CI tests are gonna be disabled in the next release because v1 itself is deprecated. There is no reason to maintain that functionality.

@TimeToBuildBob

Copy link
Copy Markdown
Contributor Author

Agreed — closing this workaround in favor of removing the deprecated v1 CI coverage in the next release. No need to maintain a separate UNI-exchange xfail.

liquid-8 pushed a commit that referenced this pull request Oct 12, 2026
… grant D3/D4) (#480)

* feat(metrics): adoption baseline snapshot script and daily workflow

Adds scripts/adoption_snapshot.py (stdlib-only) that captures:
- GitHub stars, forks, watchers, open issues
- GitHub Traffic: clones/views (14-day window; requires push-access token
  — the bundled GH Actions workflow captures this automatically)
- PyPI downloads last day/week/month (pypistats.org)
- Recent release versions and dates

Modes:
  Snapshot (default): emits metrics/adoption-YYYY-MM-DD.{json,md}
  Report (--report BASELINE.json): prints deltas vs baseline for D4

Also adds .github/workflows/adoption-snapshot.yml (daily at 06:00 UTC)
that runs with the repo's GITHUB_TOKEN so traffic data accumulates before
the 14-day window rolls off.

Initial bootstrap snapshot included (2026-09-02): 1011 stars, 375 forks,
1044 PyPI downloads/month.

Per-version download splits require BigQuery (documented in script TODO).

Co-authored-by: Erik Bjäreholt <erik@bjareho.lt>
Git-Session-Id: 3b38

* fix(metrics): fail closed on traffic auth and collector errors

The default Actions GITHUB_TOKEN cannot read /traffic/* (always 403).
The workflow now prefers ADOPTION_GITHUB_TOKEN (PAT), runs --strict so
failed/skipped collectors do not commit N/A snapshots, retries 429s,
and includes traffic deltas in --report. Push path rebases on conflict.

* test: xfail v1 UNI-exchange price-output cases (no liquidity)

Same fix as #479 — the v1 UNI exchange has no remaining liquidity so
get_price_output raises EVM error InvalidJump on mainnet fork.

Git-Session-Id: 8f77

* fix(metrics): reconstruct traffic period totals from daily series

GitHub Traffic is a rolling 14-day window, not a cumulative counter.
Subtracting two windows produced misleading D4 deltas. Store the daily
breakdown (per=day) and union it across snapshots for period clone/view
counts. Unique visitors stay point-in-time — they are not additive.

* fix(metrics): exclude partial current UTC day from period totals

Report mode included today's still-accumulating GitHub Traffic row in
D4 period totals, understating clones/views on every 06:00 UTC run.
Drop that day from the reconstructed range and label it excluded.

* fix(metrics): pin collection clock so midnight cannot include a partial day

complete_period_end() re-read datetime.now() after collect_metrics() had
already stamped snapshot_at. A report that started before UTC midnight and
finished after kept the earlier snapshot day, skipped exclusion, and
published that day's still-accumulating Traffic row as a complete total.

Use one collected_at for snapshot_at, the filename, and period-end
exclusion. When now is omitted, key off the snapshot day rather than a
fresh clock.

---------

Co-authored-by: Erik Bjäreholt <erik@bjareho.lt>
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