fix: resume spec test downloads and show progress#9217
fix: resume spec test downloads and show progress#9217shrirajpawar4 wants to merge 2 commits intoChainSafe:unstablefrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a resumption mechanism for spec test downloads by tracking progress in a .download-state directory and using marker files for completed extractions. It also adds a DownloadProgressReporter to provide visual progress bars in TTY environments and milestone-based logging elsewhere. A bug was identified where a malformed content-length header could result in a NaN value, leading to a RangeError during progress bar rendering; a suggestion was provided to validate the parsed value before use.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e264513642
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!stateDirExists) { | ||
| for (const test of testsToDownload) { | ||
| fs.closeSync(fs.openSync(getDoneMarkerPath(stateDir, test), "w")); | ||
| } |
There was a problem hiding this comment.
Re-check archive completeness before auto-migrating state
This migration path assumes that a matching version.txt with no .download-state always means a fully downloaded legacy dataset, then writes .done markers for every archive. If the hidden state directory is missing for any other reason (for example, copied artifacts that dropped dot-directories or cleanup scripts removing hidden folders), the downloader will silently skip required archives and report the version as complete even when test data is missing. Please validate each archive's extracted content (or force re-download) before creating all done markers.
Useful? React with 👍 / 👎.
Motivation
download-spec-testsstill had the original#6991behavior onunstable:same-version interrupted runs were not resumable because completion was tracked
only by a final root
version.txt, and download progress was not shown in auseful way.
Description
This change fixes the downloader in
@lodestar/spec-test-utilby:.download-statedirectoryversion.txtfor compatibility*.tar.gz.partand only marking an archive complete after extraction succeeds.download-stateduring spec test iteration so existing spec discovery is unaffectedIt also adds targeted unit coverage for:
version.txtmigrationCloses #6991
AI Assistance Disclosure
I used Codex to inspect the existing downloader, compare it with the stale PR,
design the fix, and generate the initial implementation and tests. I reviewed the final
changes manually.