added the missing tests (and verified they pass)

[patrizzzz]

All Submissions:

• Have you followed the guidelines stated in CONTRIBUTING.md file?
• Have you checked to ensure there aren't any other open Pull Requests for the desired changed?

Type of change

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

New Feature Submission:

• Does your submission pass the tests?
• Have you linted your code locally prior to submission?

Changes To Core Features:

• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your core changes, as applicable?
• Have you successfully ran tests with your changes locally?

After these steps, you're ready to open a pull request.

a. Give a descriptive title to your PR.

b. Describe your changes.

c. Put `closes #XXXX` in your comment to auto-close the issue that your PR fixes (if such).

[greptile-apps]

Greptile Summary

This PR replaces two placeholder # TODO test stubs with real implementations: test_compile_state in test_compiler_utils.py (now two tests that verify compile_state correctly resolves async computed vars both with and without a running event loop), and test_app_with_optional_endpoints in test_app.py (now verifies the upload Route and Mount are registered on app._api).

Key observations:

• The two compile_state tests correctly exercise both code paths in compile_state() — the asyncio.run(...) path and the ThreadPoolExecutor path.
• The async-loop variant (test_compile_state_resolves_async_computed_vars_with_running_event_loop) is missing assertions for regular state vars a and b that its synchronous counterpart includes, making coverage slightly inconsistent.
• test_app_with_optional_endpoints does not wrap Upload.is_used = False cleanup in a try/finally block; if either assertion fails mid-test the flag remains True for subsequent tests, which could cause hard-to-diagnose failures. Using monkeypatch would guarantee proper teardown.

Confidence Score: 4/5

• Safe to merge with minor improvements; the new tests are a clear improvement over the stubs and the issues found are non-blocking style concerns.
• Both files only add test code (no production changes). The logic under test is sound, but the fragile cleanup in test_app_with_optional_endpoints and the incomplete assertions in the async-loop compiler test reduce confidence slightly.
• tests/units/test_app.py — the Upload.is_used cleanup should be in a try/finally block to prevent test pollution on failure.

Important Files Changed

Filename	Overview
tests/units/compiler/test_compiler_utils.py	Replaces placeholder TODO stub with two real tests for compile_state: one for no event loop, one for a running async event loop. The async computed var test with running event loop omits assertions for regular state vars (a, b) that the non-async variant includes.
tests/units/test_app.py	Replaces the TODO stub for test_app_with_optional_endpoints with a real implementation that verifies upload Route and Mount are registered. The Upload.is_used = False cleanup after the with-block is not inside a try/finally, so failures leave Upload.is_used dirty for subsequent tests.

Sequence Diagram

sequenceDiagram
    participant T1 as test_no_loop
    participant T2 as test_running_loop
    participant T3 as test_optional_endpoints
    participant CS as compile_state
    participant RD as _resolve_delta
    participant App as App
    participant UE as _add_optional_endpoints

    T1->>CS: compile_state(CompileStateState)
    CS->>CS: get_running_loop raises RuntimeError
    CS->>RD: asyncio.run(_resolve_delta)
    RD-->>CS: resolved dict with async_value=resolved
    CS-->>T1: sorted compiled dict

    T2->>CS: compile_state(CompileStateState)
    CS->>CS: get_running_loop returns loop
    CS->>RD: ThreadPoolExecutor submits asyncio.run(_resolve_delta)
    RD-->>CS: resolved dict with async_value=resolved
    CS-->>T2: sorted compiled dict

    T3->>App: App() inside chdir(tmp_path)
    App-->>T3: app with _api set
    T3->>UE: Upload.is_used=True then _add_optional_endpoints
    UE->>UE: add_route POST /_upload
    UE->>UE: mount /_upload as uploaded_files
    UE-->>T3: routes registered
    T3->>T3: assert Route and Mount present

Loading

_{Last reviewed commit: e9e97c8}

[greptile-apps]

Fragile cleanup – Upload.is_used not reset on test failure

Upload.is_used = False is placed after the with chdir(tmp_path): block without a try/finally guard. If either assert any(...) call on lines 1688–1696 raises an AssertionError, the cleanup line never executes and Upload.is_used is left as True for subsequent tests. Wrap the assertions in try/finally or, preferably, use the existing monkeypatch fixture for test-scoped teardown:

Suggested change

	)
	Upload.is_used = False
	finally:
	Upload.is_used = False

Or, restructure the test to use the monkeypatch fixture:

def test_app_with_optional_endpoints(tmp_path: Path, monkeypatch):
    from reflex.components.core.upload import Upload
    from starlette.routing import Mount, Route

    monkeypatch.setattr(Upload, "is_used", False)
    with chdir(tmp_path):
        app = App()
        monkeypatch.setattr(Upload, "is_used", True)
        app._add_optional_endpoints()
        ...

Using monkeypatch guarantees restoration to the original value regardless of how the test exits.

[masenf]

i suggest setting Upload.is_used with monkeypatch inside a fixture that this test depends on, that way the clean up and test logic are cleanly separated

[greptile-apps]

Good call – that's cleaner. Here's how it would look:

Suggested change

	)
	Upload.is_used = False
	@pytest.fixture
	def upload_component(monkeypatch):
	from reflex.components.core.upload import Upload
	monkeypatch.setattr(Upload, "is_used", False)
	yield Upload
	def test_app_with_optional_endpoints(tmp_path: Path, upload_component):
	from starlette.routing import Mount, Route
	with chdir(tmp_path):
	app = App()
	upload_component.is_used = True
	app._add_optional_endpoints()
	assert app._api is not None
	upload_path = str(constants.Endpoint.UPLOAD)
	routes = list(app._api.routes)
	assert any(
	isinstance(r, Route)
	and r.path == upload_path
	and "POST" in getattr(r, "methods", set())
	for r in routes
	)
	assert any(
	isinstance(r, Mount) and r.path == upload_path and r.name == "uploaded_files"
	for r in routes
	)

The fixture ensures Upload.is_used is False initially and handles cleanup automatically, while the test logic stays focused on the actual behavior being tested.

[greptile-apps]

Actually, I can't provide this as an inline suggestion because:

1. The fixture needs to be placed before the test function
2. The comment is only on lines 1697-1698 (the function signature)
3. Inline suggestions only modify the exact lines the comment is attached to

Here's the pattern you should use:

@pytest.fixture
def upload_enabled(monkeypatch):
    """Fixture that enables Upload and cleans up afterward."""
    from reflex.components.core.upload import Upload
    
    monkeypatch.setattr(Upload, "is_used", True)
    yield
    monkeypatch.setattr(Upload, "is_used", False)


def test_app_with_optional_endpoints(tmp_path: Path, upload_enabled):
    from reflex.components.core.upload import Upload
    from starlette.routing import Mount, Route

    with chdir(tmp_path):
        app = App()
        app._add_optional_endpoints()

        assert app._api is not None
        upload_path = str(constants.Endpoint.UPLOAD)
        routes = list(app._api.routes)

        assert any(
            isinstance(r, Route)
            and r.path == upload_path
            and "POST" in getattr(r, "methods", set())
            for r in routes
        )
        assert any(
            isinstance(r, Mount) and r.path == upload_path and r.name == "uploaded_files"
            for r in routes
        )

Place the upload_enabled fixture above the test function. The fixture handles both setup and teardown automatically, keeping the test logic clean.

[greptile-apps]

Missing assertions for regular state vars in async-loop test

The synchronous counterpart (test_compile_state_resolves_async_computed_vars_without_event_loop) asserts that both regular state vars (a, b) and the async computed var are resolved correctly. This test only checks async_value, leaving the regular-var assertions untested for the running-event-loop code path. Consider adding the same assertions for completeness and consistency:

Suggested change

	@pytest.mark.asyncio
	async def test_compile_state_resolves_async_computed_vars_with_running_event_loop():
	compiled = compile_state(CompileStateState)
	values = _get_state_values(compiled, CompileStateState)
	assert values[f"async_value{FIELD_MARKER}"] == "resolved"
	@pytest.mark.asyncio
	async def test_compile_state_resolves_async_computed_vars_with_running_event_loop():
	compiled = compile_state(CompileStateState)
	values = _get_state_values(compiled, CompileStateState)
	assert values[f"a{FIELD_MARKER}"] == 1
	assert values[f"b{FIELD_MARKER}"] == 2
	assert values[f"async_value{FIELD_MARKER}"] == "resolved"

[masenf]

thanks for adding the test cases

[masenf]

i suggest setting Upload.is_used with monkeypatch inside a fixture that this test depends on, that way the clean up and test logic are cleanly separated