ci: add unit-test workflow + single-source __version__ - #22
Merged
Conversation
Until now only release.yml existed, triggered by version tags. Every other commit — including all PRs — carried no status check, so a red commit could land in main if someone forgot to run pytest locally. The v0.2.1 release shipped with __version__ still at 0.1.0; that kind of drift is exactly what a CI gate catches. ci.yml runs the unit suite on ubuntu-latest / Python 3.11 on push to main and on every PR. Integration tests that hit the live ddgs pipeline are deselected with -m "not integration" so the check is a stable signal: upstream rate-limiting or a transient network blip must never turn a correct commit red. Cross-platform coverage stays with release.yml, which already builds and smoke-tests the PyInstaller binary on windows/ubuntu/macos.
test_tools_call_returns_json_shaped_response is the one test that runs the real ddgs/DuckDuckGo pipeline — every other test is offline (pure logic or fake-injected seams). Without a marker, the new ci.yml would have to deselect by test name (-k 'not ...'), which breaks the moment the test is renamed. Register the marker in pyproject.toml so it's documented and doesn't warn. Local `python -m pytest` still runs the full 25 — CI is the only place that deselects (-m "not integration"). Developers keep a way to exercise the live network path when they need to.
v0.2.1 shipped while both pyproject.toml and __init__.py still read
0.1.0 — two hardcoded sources with no sync mechanism, so the release
tag and the in-code version drifted apart silently.
__init__.py now reads the version via importlib.metadata, making
pyproject.toml the single source of truth. __version__, the MCP
serverInfo.version reported by server.py, and the PyInstaller-built
binary all follow pyproject automatically once installed. Bumps
pyproject to 0.2.1 to match the current release.
The PackageNotFoundError fallback ("0.0.0+dev") keeps running from a
raw checkout without `pip install -e .` from crashing — version isn't
meaningful in that mode, but tests and dev workflows that don't read
it still work.
Also updates agent-web-search.spec to bundle our own dist-info via
copy_metadata(). Without this, the frozen binary would not carry the
package metadata and importlib.metadata.version() would raise
PackageNotFoundError at runtime — silently falling back to
"0.0.0+dev" in every release. The spec change is what makes the
single-source claim actually hold for the distributed binary.
The smoke test only checked serverInfo.name, so a binary that silently fell back to __version__ = '0.0.0+dev' (the PackageNotFoundError branch in __init__.py) would pass smoke and ship. That happens precisely when package metadata isn't bundled — the exact regression the spec's copy_metadata() guards against. Now asserts version is present and doesn't look like the dev fallback. If the spec or the __init__.py single-sourcing ever regresses, the release smoke catches it before the binary goes out the door.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
test_stdout_is_clean_json_rpc_only manually closed proc.stdin before calling proc.communicate(). communicate() closes stdin itself and then calls stdin.flush() internally; on a pre-closed file that flush raises ValueError: flush of closed file on Linux, while Windows tolerates it. This was a latent bug masked by developing on Windows — the new ci.yml runs on ubuntu and surfaced it immediately as a hard failure. That's exactly what CI is for. The manual close was redundant anyway: communicate() handles stdin closure by default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Two gaps surfaced during the v0.2.1 wrap-up:
release.ymlexisted (tag-triggered). Every push to main and every PR carried no ✓/✗ — a red commit could land if someone forgot to run pytest locally.pyproject.tomland__init__.pystill read0.1.0. Two hardcoded sources, no sync mechanism.What (4 commits)
799d4e9ci: add workflow —.github/workflows/ci.ymlruns the unit suite on push to main + PRs, ubuntu-latest / Python 3.11. Integration tests deselected (-m "not integration") so upstream network blips never turn a correct commit red.163298btest: integration marker —@pytest.mark.integrationon the one network-dependent test (test_tools_call_returns_json_shaped_response), marker registered inpyproject.toml. Localpyteststill runs all 25; CI deselects 1.11e55d7fix(version): single-source —__init__.pyreads__version__viaimportlib.metadata.version();pyproject.tomlbumped to0.2.1. Also updatesagent-web-search.spectocopy_metadata("agent-web-search")— without this the frozen binary loses package metadata and silently falls back to0.0.0+dev.3e61f80test(smoke): version assertion —scripts/smoke_test.pynow assertsserverInfo.versionis present and doesn't look like the dev fallback, so a broken release can't ship with a garbage version.Verification
python -m pytest -m "not integration"→ 24 passed, 1 deselectedpython -m pytest(full) → 25 passedpip install -e . && python -c "import agent_web_search; print(agent_web_search.__version__)"→0.2.1smoke test OK: 1.75s startup, server=agent-web-search v0.2.1— confirmscopy_metadatamadeimportlib.metadataresolve in the frozen binary (the key risk the code-review caught)The code-review catch worth calling out
The review subagent flagged that
importlib.metadata.version()depends on.dist-infothat PyInstaller doesn't bundle by default. Withoutcopy_metadatain the spec, every release binary would reportserverInfo.version = "0.0.0+dev"and the existing smoke test wouldn't catch it (it only checkedserverInfo.name). Commits 3 and 4 close both halves of that gap.What this PR does NOT do
main→ Require status checks to pass before merging → selecttest(the ci.yml job name).release.ymlalready covers windows/ubuntu/macos for the binary; CI is intentionally logic-only.Post-merge
The first push to main after merge will trigger ci.yml and you'll see the green ✓ on the merge commit. If you want the gate enforced on future PRs, enable branch protection as above.