Skip to content

[Backport release/0.0.14] build: fetch LFS test data before the suites run - #4181

Open
github-actions[bot] wants to merge 1 commit into
release/0.0.14from
backport/4106-to-release/0.0.14
Open

github-actions[bot] wants to merge 1 commit into
release/0.0.14from
backport/4106-to-release/0.0.14

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Description

Backport of #4106 to release/0.0.14.

@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

Not safe to merge: affected tests can still perform the timeout-prone LFS download during test execution.

Findings

  1. P1 Dynamic archives omitted

Summary

The new prefetch helper moves many LFS test-data downloads ahead of the pytest launchers, but it misses archives whose filenames are assembled with f-strings. The affected occupancy tests can still download their data while running, so this is not safe to merge until those archives are included.

Reviews (1) · Last reviewed commit: "build: fetch LFS test data before the su..."

Comment thread bin/fetch-test-data
Comment on lines +7 to +9
if grep -rqF -e "\"$n\"" -e "'$n'" --include='*.py' dimos; then
include+=("$f")
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Dynamic archives omitted

The selector only includes an archive when its complete filename appears as a quoted Python literal. The occupancy tests construct filenames from palette or method, so their existing archives—including visualize_occupancy_rainbow.png, resample_path_simple.png, and gradient_voronoi.png—are not prefetched. When those files are absent locally, get_data() still downloads them during the test, leaving the download-delay timeout this change is intended to avoid.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Artifacts

Dynamic asset selection validation script

  • The executable script reproduces the changed archive-selection predicate against dynamically generated test-data names.

Dynamic asset selection validation output

  • The captured output shows that all six existing dynamic asset archives are excluded by the selector.

View artifacts

T-Rex Ran code and verified through T-Rex

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