Skip to content

Harden test suites#89

Open
evilkermit wants to merge 8 commits into
galaxyproject:mainfrom
evilkermit:main
Open

Harden test suites#89
evilkermit wants to merge 8 commits into
galaxyproject:mainfrom
evilkermit:main

Conversation

@evilkermit

Copy link
Copy Markdown

Description

I've added the functionalities described in #80 and bumped a dependency in the Python dependencies to fix a warning in the test suite.

I'm not 100% that the way I've handled the completeness test is appropriate, but that should be pretty easy to rework as needed.

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 cause existing functionality to not work as expected)
  • 📚 Documentation update
  • ⬆️ Dependency update
  • 🧰 Maintenance/chore

Checklist

  • I have performed a self-review of my code
  • I have added tests for my changes
  • I have updated the documentation accordingly
  • My changes generate no new warnings

Related Issues

Closes #80

@dannon

dannon commented Jul 20, 2026

Copy link
Copy Markdown
Member

Nice work on this, John! The two-way parity check is exactly the right fix -- adding the fixture − INTENTIONAL_GAPS ⊆ implemented direction closes the silent-gap hole that let Phase 1 ship 14/37 ops green, and pulling INTENTIONAL_GAPS out into a shared intentional-gaps.json means it's a single source of truth now instead of two hand-kept copies. Dropping the hardcoded EXPECTED list in completeness.test.ts in favor of deriving from the fixture is great too. I ran the full TS workspace locally and everything's green (galaxy-ops 271, galaxy-cli 14, galaxy-mcp 7).

One thing I'd like to sort out before merging: the httpxhttpx2 swap in pyproject.toml. It's unrelated to #80 (which is TS-only), and I'm not sure it actually does what the commit says. Nothing in mcp-server-galaxy-py imports httpx, there's no filterwarnings = error in the pytest config so a DeprecationWarning wouldn't fail the suite anyway, and the real httpx 0.28.1 stays in the lock as a transitive dep of fastmcp/mcp/bioblend/tuspy. So swapping the (unused) dev pin to httpx2 shouldn't silence any warning -- it just adds a brand-new HTTP stack (httpx2 + httpcore2 + truststore) to the dev tree. What was the warning you were seeing, and did httpx2 actually make it go away? My hunch is this commit can just be dropped, but happy to be wrong.

Couple of minor notes, non-blocking:

  • The CLI expect(names.length).toBe(allOperations.length) is basically tautological now, since buildProgram iterates allOperations to build the commands -- it'll always match. That's fine and it's what the issue asked for; just flagging that the real "right number of ops" guard now lives in the completeness test, which is the better place for it anyway.
  • The new "fixture name parity with the registry" check in parity.test.ts and the fixtureDrift assertion in completeness.test.ts are the same check (toolNames() is just allOperations.map(op => op.name)), so there's a little overlap across the two packages. Harmless -- each suite staying self-contained is defensible.
  • The branch is a few commits behind main, so it'll want a rebase before merge. Low conflict risk since main's recent commits are npm/docs/release stuff.

Overall the test hardening is solid and I'm happy to approve it -- just want to figure out the httpx2 change first.

@evilkermit

Copy link
Copy Markdown
Author

I've reverted the httpx2 change. I've honestly forgotten exactly why I included it at this point; I think there was a warning on my local copy when running the tests. I don't remember that being a major issue, though, so it could certainly be dealt with at some point in the future if it becomes necessary to do this upgrade.

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.

galaxy-ops: test hardening -- derive op lists + two-way parity

2 participants