Skip to content

Restore real CI coverage for SQLServerPersister (needs a seeded SQL Server service) #2

Description

@thorwhalen

Restore real CI coverage for SQLServerPersister (needs a seeded SQL Server)

Context

During the wads rollout, odbcdol/tests/test_simple.py was skip-guarded so CI could go green: SQLServerPersister() needs a live SQL Server and the test currently skips when none is reachable. So odbcdol has no real test coverage running in CI today (only import + the check_requirements doctests execute).

Unlike the other data-backends we just made self-contained (redisdol/redisposted → services: redis, s3dol → services: motoserver/moto, sshdol → localhost sshd, mongodol → services: mongo), SQL Server is heavier and this test can't just be pointed at a service as-is.

Why it's not a drop-in service-container fix

test_simple.py::test_sqlserver_persister has no assertions and assumes a pre-seeded person table (it does sql_server_persister[1], [3], iterates, len(...)). To exercise it in CI we'd need all of:

  1. a services: mssql container (mcr.microsoft.com/mssql/server, ACCEPT_EULA=Y, SA_PASSWORD, ~1.5 GB image, slow startup + health wait);
  2. the ODBC driver installed on the runner (msodbcsql18 + unixodbc — [tool.wads.ops.*] already declares these, but msodbcsql18 has EULA/apt friction, and the persister hardcodes DRIVER={ODBC Driver 17 for SQL Server});
  3. a CI seed step creating the py2store/person schema + rows the test reads;
  4. a rewrite of test_simple.py into a proper self-seeding test with real assertions (create table → write → read-back-assert → delete → assert), so it doesn't depend on ambient data.

Recommendation

Do (1)–(4) as a focused task: add the mssql service to the inline uv CI (pattern: i2mint/redisdol .github/workflows/ci.yml), a seed step, and convert test_simple.py to a self-contained CRUD round-trip with assertions. The conftest can keep an availability-deselect gate so the local gate stays green without a server (like mongodol).

Until then it stays skip-guarded (CI green, but this backend's core store is unexercised).

Filed from the wads repo-improvement rollout (thorwhalen/priv#14).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions