Skip to content

tests: check that client and server schemas match#1192

Open
HirenGajjar wants to merge 1 commit into
mlco2:masterfrom
HirenGajjar:tests/schema-compatibility-check
Open

tests: check that client and server schemas match#1192
HirenGajjar wants to merge 1 commit into
mlco2:masterfrom
HirenGajjar:tests/schema-compatibility-check

Conversation

@HirenGajjar
Copy link
Copy Markdown

@HirenGajjar HirenGajjar commented May 13, 2026

Description

Adds a test suite that verifies the client (codecarbon/core/schemas.py) and server (carbonserver/carbonserver/api/schemas.py) schemas stay compatible, addressing #1190.

What it checks

For each shared model pair (EmissionBase, RunBase, ExperimentBase, ProjectBase, OrganizationBase) — 15 parametrized tests total:

  1. Field presence — every client field exists in the server schema
  2. Required fields — every required server field exists in the client schema
  3. Type compatibility — shared fields have wire-compatible types

Why ast instead of imports

The client and server live in separate uv environments. Using Python's ast module to parse schema files as source avoids any import or path issues — the test runs in the existing client test environment with zero extra dependencies.

Regression proof

This test catches the exact bug fixed in #1189. Temporarily changing on_cloud: Optional[bool] to Optional[str] on the server produces:

FAILED test_shared_field_types_are_compatible[ExperimentBase-...]
AssertionError: [ExperimentBase] Incompatible types between client and server schemas:
  on_cloud: client='bool'  server='Optional[str]'

Related Issue

Closes #1190

Types of changes

  • 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 change)

AI Usage Disclosure

  • 🟥 AI-vibecoded: You cannot explain the logic.
  • 🟠 AI-generated: Car analogy: the car drives itself, you are inside and give instructions.
  • ⭐ AI-assisted. Car analogy: you drive the car, AI helps you find your way.
  • ♻️ No AI used.

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the contributing.md document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@HirenGajjar HirenGajjar requested a review from a team as a code owner May 13, 2026 05:25
Copy link
Copy Markdown

@MatthewJamisonJS MatthewJamisonJS left a comment

Choose a reason for hiding this comment

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

Pulled this down, all 15 pass in <0.1s. Test catches the #1189 class of bug cleanly. One thing I think is worth fixing before it lands, plus a couple smaller notes.

The optionality axis isn't covered. _types_compatible("Optional[str]", "str") returns True, so if a client field is Optional[str] = None and the corresponding server field is required str, the test passes but the client can still send None and the server will reject it. Same shape of bug as #1189 just on a different axis. You're already parsing required so it's basically:

@pytest.mark.parametrize("label,client_cls,server_cls", SCHEMA_PAIRS)
def test_required_alignment_on_shared_fields(label, client_cls, server_cls):
    client = _parse_class_fields(CLIENT_SCHEMA_PATH, client_cls)
    server = _parse_class_fields(SERVER_SCHEMA_PATH, server_cls)
    weakened = [
        f for f in set(client) & set(server)
        if server[f]["required"] and not client[f]["required"]
    ]
    assert not weakened, f"[{label}] server requires fields client treats as optional: {weakened}"

Smaller stuff:

_unwrap_optional only handles the Optional[X] spelling. The moment anyone writes str | None or Union[str, None] in a schema the test will start false-failing — not silent, just noisy. Worth handling now if you want to keep it cheap to maintain.

Field() with nothing in it reads as optional today (no Ellipsis, no default=). In Pydantic v2 that's actually required. None of the current schemas use it so it's not biting, but if you're already touching the helper for the optionality fix it's a one-liner to cover.

And SCHEMA_PAIRS is (name, name, name) five times — is that there because you're expecting the server classes to get renamed (Base → Read or whatever) at some point? If so a comment would save the next person guessing, otherwise it can collapse to a list of strings.

Nothing in the second half is a blocker for what the PR actually claims to do. The required-alignment one I think is — it's the same kind of silent wire mismatch the test was written to prevent.

Add 15 parametrized tests across EmissionBase, RunBase, ExperimentBase,
ProjectBase and OrganizationBase that verify the client (codecarbon/core/schemas.py)
and server (carbonserver/carbonserver/api/schemas.py) schemas are compatible.

Tests check:
- All client fields exist in the server schema
- All required server fields exist in the client schema
- Shared fields have wire-compatible types

Uses ast to parse schema files without importing either module,
so the test runs in the client test environment with no extra deps.

This would have caught the on_cloud: str vs bool mismatch fixed in mlco2#1189.

Closes mlco2#1190
@HirenGajjar HirenGajjar force-pushed the tests/schema-compatibility-check branch from 8081f41 to fa44a54 Compare May 19, 2026 07:19
@HirenGajjar
Copy link
Copy Markdown
Author

Thanks for the thorough review. Addressed everything in the latest push:

  1. Added test_required_alignment_on_shared_fields for the optionality axis
  2. _unwrap_optional now handles str | None and Union[str, None]
  3. _is_pydantic_required_field now treats bare Field() as required (Pydantic v2)
  4. Kept SCHEMA_PAIRS as a three-tuple with an explanatory comment — the intent is to make future renames a one-line change in a single place rather than touching the test functions

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: check that client and server schemas match

2 participants