ci(docs): run docs/app tests in CI and guard Upload from being evaluated#6551
Open
adhami3310 wants to merge 13 commits into
Open
ci(docs): run docs/app tests in CI and guard Upload from being evaluated#6551adhami3310 wants to merge 13 commits into
adhami3310 wants to merge 13 commits into
Conversation
Adds a docs_tests workflow that runs `pytest tests/` in docs/app on changes to docs/**, plus a new test that loads `reflex_docs.pages` in a subprocess and asserts `Upload.is_used` stayed False — catches regressions where an `rx.upload(...)` call sneaks into an executed markdown block or a frontmatter preview lambda. Fixes the one regression the new check surfaced: the `Upload:` preview lambda in `library/forms/upload.md` frontmatter was being eval'd at route-build time, instantiating an upload component and forcing the docs site to mount the upload endpoint. Removing the lambda falls through to the non-interactive fragment fallback in `component.py:540-541`, so no Upload is constructed. Side cleanup along the way: - `test_agent_files.py` monkeypatched `reflex_base.config.get_config`, but `agent_files._plugin` does `from reflex_base.config import get_config` at module load. Patch `agent_files._plugin.get_config` instead — 3 tests had silently rotted because docs/app tests never ran in CI. - Delete unused docs/app/tests/conftest.py (Playwright fixtures + video hook used by no test), utils.py (helper imported by no test), and the orphan tests/assets/chakra_color_mode_provider.js asset.
Contributor
Greptile SummaryThis PR adds a
Confidence Score: 4/5Safe to merge; changes are well-scoped to CI infrastructure, test correctness, and a docs-build regression fix. The core regression fix (removing the Upload preview lambda) and the monkeypatch corrections are straightforward and low-risk. The new subprocess test is solid in design but omits a timeout parameter on subprocess.run — if import reflex_docs.pages ever hangs, the test silently blocks until the 20-minute job limit rather than failing quickly with a clear message. docs/app/tests/test_upload_not_evaluated.py — the missing subprocess timeout is worth addressing before this lands. Important Files Changed
Reviews (1): Last reviewed commit: "ci(docs): run docs/app tests in CI and g..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Match marketing/tests/test_routes.py: import the routes module via a pytest fixture, then assert `Upload.is_used` stayed False. No subprocess plumbing — the assertion sees the current flag, which reflects whether *any* code path constructed an Upload component during compilation, so the check works regardless of test order.
- test_breadcrumbs, test_docgen_double_eval, test_routes: imports
resolve through the workspace venv (`reflex_docs` is installed as a
workspace member), so `sys.path.append('docs/app')` was redundant.
- test_doc_links: `check_doc_links` is a bare script at
docs/app/scripts/, not a package. Load it via `importlib.util` from
its file path instead of poking sys.path.
`docs/app/scripts/check_doc_links.py` was a bare script that test_doc_links.py could only import via sys.path or importlib.util gymnastics. Move it into the installed `reflex_docs` package so the test (and anyone else) can `from reflex_docs.scripts.check_doc_links import ...` directly. - Move the file to `docs/app/reflex_docs/scripts/check_doc_links.py` with an `__init__.py` so it's a proper package. - Update the default `--md-root` / `--sitemap` paths since the script is two directories deeper now. - Switch the integration workflow to `python -m reflex_docs.scripts.check_doc_links`. - Drop the importlib spec/loader block from test_doc_links.py in favour of a normal import.
…from pytest The script's only consumer was a workflow step; nothing ran it interactively. Strip `main()` + the argparse/sys imports and keep the module as a library only. Add `test_docs_links_against_exported_sitemap` to test_doc_links.py: it calls `check()` against the real docs root and the exported sitemap, and `pytest.skip`s when `.web/public/sitemap.xml` is absent so `pytest tests/` still passes without a build. Switch the integration workflow step to `uv run pytest tests/test_doc_links.py`.
Wire the link-check pytest into the `reflex-docs` job in the root integration_tests workflow: the existing prod-mode integration.sh already builds the site (populating .web/public/sitemap.xml), so a follow-up `pytest tests/test_doc_links.py` runs against a real sitemap instead of skipping. Also delete docs/app/.github/workflows/integration_tests.yml. That file was a leftover from when docs/app shipped as its own repository — GitHub Actions only reads .github/workflows/ at the repo root, so the file (and its sibling codespell.yml/pre-commit.yml/unit_tests.yml) has never run in this monorepo. The /docs link validation it described is now hosted by the active workflow above.
Skipping the link test is fine for `pytest tests/` on a dev machine that hasn't run an export, but in CI a missing sitemap means the workflow forgot to build the docs — silently skipping there hides a broken pipeline. Promote the skip to `pytest.fail` when the `CI` env var is set (GitHub Actions, GitLab, CircleCI, etc. all set it).
Use a pytest primitive instead of the env-var branching: mark the end-to-end link test xfail (run=False) when sitemap.xml is absent, and pass --runxfail in the CI step. Locally `pytest tests/` still no-ops the test when the docs haven't been built; in CI --runxfail neuters the marker so a missing sitemap surfaces as a real failure instead of a silent skip.
The link checker's only consumer is its own test file; a dedicated module under reflex_docs.scripts was extra indirection for no benefit. Inline the helpers (`_normalize`, `_walk_blocks`, `check`, etc.) at the top of tests/test_doc_links.py and delete docs/app/reflex_docs/scripts/ entirely.
`uv sync` was running from the repo root, which doesn't install the docs/app workspace member (it isn't listed in the root `[tool.uv.sources]`), leaving `agent_files` off sys.path and breaking test collection in docs_tests.yml. Add a `working-directory` input to the action so callers can point uv at the right workspace project, and set it to `./docs/app` in docs_tests.yml.
router_attributes.md linked to `/docs/pages/dynamic_routing` in two places. The source file is `docs/pages/dynamic_routing.md` but the served URL is `/docs/pages/dynamic-routing/` (docgen rewrites underscores to hyphens). Switch the links to the hyphenated form so they resolve.
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.
Summary
docs_testsworkflow that runspytest tests/indocs/appon changes todocs/**, the upload component, or the workflow itself — docs tests previously ran in nobody's CI.tests/test_upload_not_evaluated.py: a single subprocess-based test that importsreflex_docs.pagescold and assertsUpload.is_usedis stillFalse. Catchesrx.upload(...)calls leaking into an executed markdown block (python exec/python demo/python demo exec/python eval) or a frontmatter preview lambda.Upload:preview lambda indocs/library/forms/upload.mdfrontmatter was being eval'd at route-build time, instantiating an upload component and forcing the docs site to register the upload HTTP endpoint. Removing the lambda falls through to the non-interactive fragment fallback incomponent.py:540-541, so no Upload is constructed during build.Why this matters
Upload.is_usedflips toTruethe momentrx.upload(...)/rx.upload.root(...)is constructed. The reflex app reads that flag (and an on-disk marker it writes) to decide whether to register the/_uploadendpoint. The docs site never needs uploads, so any code path that builds an Upload component during compilation is a bug that leaves the docs site serving an unused endpoint.Drive-by cleanup
docs/app/tests/test_agent_files.py: 5monkeypatch.setattrcalls targetedreflex_base.config.get_config, butagent_files._plugindoesfrom reflex_base.config import get_configat module load, so the local name was never being patched. Repointed toagent_files._plugin.get_config. Three tests had silently rotted because docs/app tests didn't run in CI.docs/app/tests/conftest.py(Playwright fixtures + video-recording hook, used by no test),docs/app/tests/utils.py(helper imported by no test), and the orphandocs/app/tests/assets/chakra_color_mode_provider.js.Test plan
cd docs/app && uv run pytest tests/passes locally (315 passed, 1 skipped).cd docs/app && uv run pytest tests/test_upload_not_evaluated.pypasses.test_upload_not_evaluatedfail with the expected message.docs testsworkflow runs and goes green on this PR.library/forms/uploaddocs page still renders correctly without theUpload:preview lambda (the auto-generated preview box for the Upload component will fall back to an empty fragment).