Skip to content

ci: add stitch-models tests & integrate helper utility for test execution#24

Open
mbarlow12 wants to merge 2 commits intomainfrom
ci/stitch-models-tests
Open

ci: add stitch-models tests & integrate helper utility for test execution#24
mbarlow12 wants to merge 2 commits intomainfrom
ci/stitch-models-tests

Conversation

@mbarlow12
Copy link
Contributor

Summary

The PR updates our python testing Makefile targets to leverage a script instead of calling uv run ... directly. It also updates the tests in stitch-models and integrates them into our CI (via the updated testing workflow).

Rationale

Current test execution relies on first calling uv sync --all-packages --dev. While this guarantees that all dependencies will appear in the virtual environment, it also hides errors if dependencies are not configured correctly within a member package. For example, if stitch-api relies on the new stitch-ogsi package but fails to declare it in its pyproject.toml, tests will pass or we won't see errors since stitch-ogsi will be synced to the virtual environment.

By leveraging the --exact flag, we effectively sync each package individually and only test against explicitly declared dependencies.

Added files

  • scripts/test-package.py minimal executable python script that accepts a package argument and optional --exact flag, discovers member packages directories, and calls uv run --package <package name> --exact --group dev path/to/package_dir

Key Changes

  • Separate uv-test and uv-test-isolated make targets. Both support a pkg=<package name> for running against a specific package and default to running all discovered tests. uv-test is useful for quick local testing, while uv-test-isolated is more robust and intended for use in out CI
  • Both tests use the new test-package.py script

AI

  • Planning and spec design
  • Wrote Makefile changes and script contents per spec
  • Implemented API usage in stitch-models test per latest package interface

@mbarlow12 mbarlow12 marked this pull request as ready for review March 2, 2026 22:03
@mbarlow12 mbarlow12 requested a review from AlexAxthelm March 2, 2026 22:04
Copy link
Collaborator

@AlexAxthelm AlexAxthelm left a comment

Choose a reason for hiding this comment

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

Actual test content looks good.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The changes to the Makefile seem out of scope for "test stitch-models", and in general, I think it's a bit over-complicated for what we want.

I think my general tendency here would be (in a separate Makefile-cleaning PR) to change to something like test-python that would depend on test-stitch-models and test-api and all our other python testing commands (but with better target names), which would run their own exact installs

(but I'm still missing the advantages of that over just syncing the whole project)

Copy link
Collaborator

Choose a reason for hiding this comment

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

See comment on Makefile, but I think we want to keep this process simpler (i.e. not have this script)

@jdhoffa
Copy link
Contributor

jdhoffa commented Mar 4, 2026

Relates to STIT-176

@mbarlow12
Copy link
Contributor Author

I realize this is a bit mixed in scope, but it was a pretty quick change that I've been wanting for a little while. The 2 goals are:

  • convenience functions to make running python tests locally easier
  • stricter environment isolation in CI

My thoughts around this are that, in using a monorepo/workspace, getting properly isolated tests is a bit of a challenge. Additionally, as I mentioned in the description, uv sync --all-packages --dev will sync everything to the same virtual environment. So here's a concrete example:

  • package-a declares sqlalchemy as a dependency
  • package-b also depends on sqlalchemy but someone forgets to explicitly add it to its pyproject.toml
  • uv sync --all-packages will install sqlalchemy to the virtual environment and uv run pytest packages/package-b will pass its tests
  • if we run uv run --package package-b --exact --dev pytest packages/package-b, it will raise an ImportError for the missing sqlalchemy dependency

Whether we use the script in the makefile is less important, though I think it's really convenient to be able to run make uv-test and get all python unit tests without having to explicitly add them anywhere, We get strong guarantees in CI that tests pass in isolation.

In a sense, it shifts the source of truth about dependencies and module/component/package relationships to the various pyproject.toml files. It's a mature standard, that I'm comfortable relying on.

Copy link
Collaborator

@AlexAxthelm AlexAxthelm left a comment

Choose a reason for hiding this comment

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

🤔 I see where you're coming from. Makes sense that it's necessary in a monorepo (again, I think it's my hesitance to that pattern that's showing).

There's still something off to me about this pattern, but I think I won't actually know what it is until I take a more holistic look at the Makefile.

In the meantime, works for me.

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.

3 participants