Skip to content

test(mcp): resolve baseline toml path and restore mcp server integration tests (fixes #527) - #567

Open
i-am-paradox wants to merge 2 commits into
darnitdevorg:mainfrom
i-am-paradox:fix/527-mcp-server-integration-tests
Open

i-am-paradox wants to merge 2 commits into
darnitdevorg:mainfrom
i-am-paradox:fix/527-mcp-server-integration-tests

Conversation

@i-am-paradox

@i-am-paradox i-am-paradox commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #527. Restores skipped MCP server integration tests and aligns assertions with current framework tools and configuration paths:

  1. Resolves openssf-baseline.toml via importlib.resources.files("darnit_baseline") directly without dead-code fallbacks in tests/integration/test_mcp_server.py and tests/darnit/server/test_factory.py.
  2. Asserts baseline TOML file existence in both test modules (including a module-scoped autouse fixture in test_mcp_server.py) so missing configurations fail tests immediately rather than skipping silently.
  3. Marks tests/integration/test_mcp_server.py with pytest.mark.integration so CI (pytest tests/ -m integration) discovers and executes the test module.
  4. Updates expected_tools in test_server_starts_and_lists_tools from 18 to 22, including harness-loop (run_next_action, submit_action_result) and judgment (submit_judgment, confirm_pass_candidate) tools.
  5. Updates test_audit_with_tags_filter to assert on each returned control's actual domain definition against openssf-baseline.toml controls table rather than relying on control ID substring matching.
  6. Adds test_get_pending_data_and_confirm_project_data testing get_pending_data and confirm_project_data round trips over MCP.

Type of Change

  • Bug fix (non-breaking change fixing an issue)

Testing

  • Tests pass locally (uv run pytest tests/ -v)
  • Added tests for new functionality (if applicable)
  • Linting passes (uv run ruff check .)

Ran locally:

  • uv run pytest tests/integration/test_mcp_server.py tests/darnit/server/test_factory.py -v: 28 passed
  • uv run ruff check tests/integration/test_mcp_server.py tests/darnit/server/test_factory.py: 0 errors
  • uv run ruff format --check tests/integration/test_mcp_server.py tests/darnit/server/test_factory.py: clean
  • uv run python scripts/validate_sync.py --verbose: all validations successful

AI assistance

  • AI assistance was used

Assisted by Antigravity (Gemini 2.5 Pro) for tracing test skips and drafting test assertions. All changes reviewed, verified, and tested against local pytest suite and sync validator.

Additional Notes

Fixes #527

@Marc-cn Marc-cn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks, this works: on main these two files give 19 passed / 8 skipped, on your branch 28 passed / 0 skipped.

Before merging:

  1. AI disclosure. AI_POLICY.md asks for the tool(s) and which parts are AI-generated, plus an Assisted-by: AGENT:MODEL trailer. You did both on #537; here the tool is not named and the commit has no trailer. Please add them.
  2. The fallback path is dead code. files("darnit_baseline") already resolves to packages/darnit-baseline/src/darnit_baseline/openssf-baseline.toml, the same file. Please drop the fallback in both files.
  3. test_audit_with_tags_filter: why hardcode STAGE1-REF-SECURITY-01? It has domain = "VM", so the filter is right and the ID check was the wrong proxy. #527 left this as an open decision. Can you explain your reasoning, and whether the test can assert on the control's domaininstead?

Optional: these tests went dark because a missing TOML is a skip. Making that a failure would stop it happening again.

Heads-up: #570 also touches tests/darnit/server/test_factory.py, so you may need to rebase.

@i-am-paradox
i-am-paradox force-pushed the fix/527-mcp-server-integration-tests branch from 7229549 to 9ee28bf Compare October 9, 2026 22:50
@i-am-paradox

Copy link
Copy Markdown
Contributor Author

Addressed review points:

  1. Rebased on current main (absorbing chore: fold darnit-example's test fixtures into darnit-testchecks and remove it (#487 part 3) #570).
  2. Added the Assisted-by: Antigravity:eni-gemini-2.5-pro trailer to the commit and updated the PR description disclosure with tool name and scope per AI_POLICY.md.
  3. Dropped the fallback paths in both files. Added assertions for baseline TOML presence (including a module-scoped autouse fixture in test_mcp_server.py) so missing configurations fail rather than skip.
  4. In test_audit_with_tags_filter, replaced the substring/ID check with a domain verification against each returned control's definition in the baseline controls table (ctrl.get("domain") or ctrl.get("tags", {}).get("domain") == "VM").

@mlieberman85 mlieberman85 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the quick turnaround. The path, marker and domain assertion all look right, and CI now runs all 8 MCP tests.

  1. The spawned darnit serve doesn't get the test isolation. tests/conftest.py monkeypatches the operator-config and data-root functions in-process, but the subprocess inherits HOME. So test_get_pending_data_and_confirm_project_data writes a real security_contact confirmation, under the local user's name, into the developer's own operator store, and every test reads the developer's ~/.config/darnit/config.toml. Could you add one fixture that builds env from mcp.client.stdio.get_default_environment() with HOME/XDG_CONFIG_HOME/XDG_DATA_HOME pointed into tmp_path, and use it for every StdioServerParameters? Launching Path(sys.executable).with_name("darnit") directly avoids uv run needing a warm cache under the new HOME. I checked that this keeps the confirmation in tmp.
  2. Please make the round trip check the effect: pass owner/repo to get_pending_data, and assert security_contact is asked for before confirm_project_data and gone after it.
  3. test_audit_with_tags_filter: add assert results so an empty result can't pass vacuously.
  4. test_factory.py::test_openssf_baseline_toml: drop the remaining except ImportError: pytest.skip(...) so a broken import fails the way a missing TOML does, and move the importlib.resources import to the top of the module.

Drafted with Claude Code; reviewed and posted by me.

…ion tests (fixes darnitdevorg#527)

- Resolve openssf-baseline.toml via importlib.resources.files without dead-code fallbacks
- Assert baseline TOML file presence so missing configs fail tests instead of skipping silently
- Mark test_mcp_server.py with pytest.mark.integration so CI integration test suite runs the module
- Update expected tools count to 22 in test_server_starts_and_lists_tools (features 025 and 041)
- Assert control domain directly in test_audit_with_tags_filter against baseline controls table
- Add test_get_pending_data_and_confirm_project_data round trip

Fixes darnitdevorg#527

Assisted-by: Antigravity:eni-gemini-2.5-pro
Signed-off-by: Paradox <159635503+i-am-paradox@users.noreply.github.com>
- Provide mcp_server_params fixture with HOME and XDG pointed into tmp_path
- Launch darnit binary directly to avoid uv cache requirements under isolated HOME
- Symlink existing tree-sitter language pack cache to prevent network fetches in tests
- Isolate test_repo directory and assert security_contact presence and removal in round trip
- Add non-empty assertion on results in test_audit_with_tags_filter
- Move importlib.resources.files to top of test_factory.py and drop ImportError skip

Assisted-by: Antigravity:eni-gemini-2.5-pro
Signed-off-by: Paradox <159635503+i-am-paradox@users.noreply.github.com>
@i-am-paradox
i-am-paradox force-pushed the fix/527-mcp-server-integration-tests branch from 9ee28bf to a939fce Compare October 10, 2026 22:51
@i-am-paradox

Copy link
Copy Markdown
Contributor Author

Addressed the review feedback in a939fce:

  1. Subprocess isolation: Added the mcp_server_params fixture building an isolated environment with HOME, XDG_CONFIG_HOME, and XDG_DATA_HOME directed into tmp_path, launching Path(sys.executable).with_name("darnit") directly. Also symlinked any existing tree-sitter-language-pack cache directory into tmp_path so spawned servers do not attempt network downloads in test environments. Applied mcp_server_params across all server invocations in test_mcp_server.py.
  2. Context round trip verification: Updated test_get_pending_data_and_confirm_project_data to pass owner="test-org" and repo="test-repo", isolated the test repository directory, and verified that security_contact is asked for in get_pending_data prior to confirmation and no longer present afterward.
  3. Tags filter non-vacuous assertion: Added assert results in test_audit_with_tags_filter before iterating through the VM domain assertions.
  4. Clean baseline test imports: Moved from importlib.resources import files to the module top in tests/darnit/server/test_factory.py and dropped the except ImportError: pytest.skip(...) so broken imports fail directly.

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.

tests/integration/test_mcp_server.py is skipped everywhere

3 participants