Skip to content

Conversation

seratch
Copy link
Member

@seratch seratch commented Oct 3, 2025

This pull request enables running all the tests in CI.

Copy link
Member Author

@seratch seratch left a comment

Choose a reason for hiding this comment

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

comments for reviewers


def __post_init__(self) -> None:
"""Validate required fields and consistency."""
if "checked_text" not in self.info:
Copy link
Member Author

Choose a reason for hiding this comment

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

The only main code change is this. This validation is not compatible with the current tests. If we think tests should be changed, I am happy to work on it later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Removing this line is fine


model = _resolve_ctx_requirements(check)
fields = getattr(model, "model_fields", getattr(model, "__fields__", {}))
# Prefer Pydantic v2 API without eagerly touching deprecated v1 attributes
Copy link
Member Author

Choose a reason for hiding this comment

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

this is to resolve warning:

tests/unit/test_registry.py::test_resolve_ctx_protocol_creates_model
  /Users/seratch/code/openai-guardrails-python/tests/unit/test_registry.py:40: PydanticDeprecatedSince20: The `__fields__` attribute is deprecated, use `model_fields` instead. Deprecated in Pydantic V2.0 to be removed in V3.0. See Pydantic V2 Migration Guide at https://errors.pydantic.dev/2.11/migration/
    fields = getattr(model, "model_fields", getattr(model, "__fields__", {}))

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

@seratch
Copy link
Member Author

seratch commented Oct 3, 2025

Ah, some tests expect OPENAI_API_KEY env variable; I didn't detect it locally. Will fix them too.

Copy link
Collaborator

@steven10a steven10a left a comment

Choose a reason for hiding this comment

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

LGTM. Code changes are fine

@gabor-openai gabor-openai merged commit 5cedce4 into main Oct 4, 2025
3 checks passed
@gabor-openai gabor-openai deleted the test-failures branch October 6, 2025 15:57
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