Skip to content

build: fetch LFS test data before the suites run - #4106

Merged
spomichter merged 10 commits into
mainfrom
krishna/fix/rel-fetch-test-data
Sep 16, 2026
Merged

spomichter merged 10 commits into
mainfrom
krishna/fix/rel-fetch-test-data

Conversation

@KrishnaH96

Copy link
Copy Markdown
Contributor

Problem

Tests pull their LFS archives lazily inside the test. On a normal link a 1.2 GB archive takes longer than the 600 s per-test timeout: the worker is killed and the session hangs. Same root cause as #2119.

Solution

bin/fetch-test-data pulls every archive a test references; pytest-slow/all/coverage call it first, like build-test-natives. Three setup lines in docs/development/testing.md (fetch, playwright browsers, -n 8 on laptops).

How to test

bin/fetch-test-data && bin/pytest-all --numprocesses=8

Lists 40 archives; no in-test LFS pull afterwards.

AI assistance

Used Fable 5.1 extensively for root cause analysis, fix and testing.

Checklist

  • I have read and approved the CLA.

@codecov

codecov Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##             main    #4106      +/-   ##
==========================================
- Coverage   79.15%   79.14%   -0.01%     
==========================================
  Files        1469     1469              
  Lines      139237   139237              
  Branches    11976    11976              
==========================================
- Hits       110212   110201      -11     
- Misses      25626    25633       +7     
- Partials     3399     3403       +4     
Components Coverage Δ
Tests 95.02% <ø> (+0.01%) ⬆️
Flag Coverage Δ
OS-ubuntu-24.04-arm 75.10% <ø> (-0.01%) ⬇️
OS-ubuntu-latest 75.81% <ø> (+0.01%) ⬆️
Py-3.10 75.58% <ø> (+<0.01%) ⬆️
Py-3.11 75.78% <ø> (ø)
Py-3.12 75.79% <ø> (+<0.01%) ⬆️
SelfHosted-Large 30.74% <ø> (-0.02%) ⬇️
SelfHosted-Linux 36.17% <ø> (+<0.01%) ⬆️
SelfHosted-macOS 35.51% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 7 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread docs/development/testing.md Outdated
paul-nechifor
paul-nechifor previously approved these changes Sep 13, 2026
@spomichter
spomichter marked this pull request as ready for review September 13, 2026 01:00
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 13, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Sep 13, 2026
spomichter
spomichter previously approved these changes Sep 13, 2026
@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

Not safe to merge until the prefetch step includes dynamically constructed test-data archive names.

Findings

  1. P1 Dynamic archives skip prefetch

Summary

  • Summary

  • The new test-data prefetch script misses archives whose names are constructed dynamically by tests. Affected tests can still start an LFS download during execution and hit their timeout or leave the test session waiting. This should be fixed before merging.

Reviews (1) · Last reviewed commit: "docs: worker count for the full suite"

Comment thread bin/fetch-test-data Outdated
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 13, 2026
@KrishnaH96
KrishnaH96 force-pushed the krishna/fix/rel-fetch-test-data branch from 295837e to 5e4ad09 Compare September 13, 2026 01:44
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Sep 13, 2026
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 13, 2026
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Sep 15, 2026
@KrishnaH96

Copy link
Copy Markdown
Contributor Author

@spomichter the merge-main commits dismissed your approval, nothing changed in the script since. Could you re-approve? Auto-merge is armed.

@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Sep 15, 2026
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Sep 15, 2026
@github-actions github-actions Bot removed the ready-to-merge Required CI checks have passed on this PR label Sep 16, 2026
@spomichter
spomichter added this pull request to the merge queue Sep 16, 2026
@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 16, 2026
Merged via the queue into main with commit 485c8c8 Sep 16, 2026
30 of 31 checks passed
@spomichter
spomichter deleted the krishna/fix/rel-fetch-test-data branch September 16, 2026 03:12
@github-actions

Copy link
Copy Markdown
Contributor

Successfully created backport PR for release/0.0.14:

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

Labels

backport release/0.0.14 ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants