Skip to content

fix: apply the download_file stub to moon_engine too (#160) - #162

Merged
LeyckerS merged 1 commit into
LeyckerS:mainfrom
XEDAB:fix-160-conftest-stub
Aug 12, 2026
Merged

fix: apply the download_file stub to moon_engine too (#160)#162
LeyckerS merged 1 commit into
LeyckerS:mainfrom
XEDAB:fix-160-conftest-stub

Conversation

@XEDAB

@XEDAB XEDAB commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Description

Close #160
Resolved the issue in #160 by modifying the erroneous line 66 in conftest.py (monkeypatch.setattr(moon_cli, "download_file", fake_download))) (which could potentially cause the engine to attempt a real download) to monkeypatch.setattr(host, "download_file", fake_download)
图片 and moved it into the for host in (moon_engine, moon_cli): loop, while adding a comment stating that this monkeypatch statement should not be moved.
This modification only involves conftest.py and does not involve any other changes.

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would change existing behavior)
  • Documentation update
  • Refactor / code cleanup
  • Other:

Checklist

  • I have tested my changes locally
  • If this affects shared logic (extraction, download engine), I also
    applied the equivalent change to moon_cli.py
  • I have kept the single-file architecture (no package split)
  • I have not added new dependencies without justification in the PR
    description

Screenshots / logs (if applicable)

图片 ↑Normal state 图片 ↑The state after commenting out lines 181 and 255

@XEDAB

XEDAB commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

By the way, I looked at line 270 mentioned in #160; it is a comment. I commented out the two lines of self._inc("ok") (181 and 255) in the fuckingfast branch, and the test crashed as expected.

@LeyckerS LeyckerS left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Correct, and verified rather than assumed:

ENGINE download_file: fake_download
CLI    download_file: fake_download

Both front-ends now hold the stub; before this, the engine held the real function. pytest tests/ -q → 50 passed.

Your verification in #160 is the part I want to point at. You did not just move the line — you went to moon_engine.py:270, found it was a comment rather than the counter, located the real increments at 181 and 255, annotated those instead, confirmed the tests went red, and restored. The line number in my issue was wrong and you worked past it without making a fuss. That is the third time you have had to correct something I wrote, and you have been right every time.

Putting the stub inside the loop rather than adding a second setattr beside it is also the right shape: the bug was that one line sat outside the loop, so the fix is to make the loop the only place any of this happens. Your comment says exactly that, which is what stops it recurring.

One cosmetic thing I am not sending you back for: that comment is a single 228-character line, against line-length = 100 in ruff.toml. It only passes because E501 sits in the ignore list, and every other comment in the file wraps. I will wrap it in the bookkeeping commit rather than cost you a round trip — the wording is yours and I am not changing it.

Merging.

@LeyckerS
LeyckerS merged commit b330922 into LeyckerS:main Aug 12, 2026
9 checks passed
LeyckerS added a commit that referenced this pull request Aug 12, 2026
@XEDAB's #162 closes #160 and @Divesh-Kshirsagar's #161 closes #151.

The comment added by #162 was a single 228-character line against the
line-length of 100 in ruff.toml; wrapped here as promised in the review
rather than sending it back. Wording unchanged, plus a pointer to #160.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LeyckerS added a commit that referenced this pull request Aug 12, 2026
Every entry in this release came from an outside contributor.

- #150 (@shard872, #116) a full disk aborts the run instead of retrying
- #149 (@Allen58562, #65) Stop interrupts transfers already in flight
- #153 (@AdvaitVarhade, #32) structured CLI exit codes
- #161 (@Divesh-Kshirsagar, #151) pytest.ini with a narrow warning filter
- #158 (@nightcityblade, #81) ruff runs once, not once per Python version
- #157 (@XEDAB, #155) the assertion that could not fail
- #162 (@XEDAB, #160) the stub that left the engine on the real network
- #159 (@AashishGupta2007, #145) the generated-era THEME block

README: consolidated the changelog sections, refreshed the stale test count,
and replaced the stars badge with a contributors badge.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

conftest stubs download_file on moon_cli only — the engine keeps the real one, and nothing currently notices

2 participants