Skip to content

Conversation

bartzwemmer
Copy link
Contributor

  • Tests pass
  • ruff format
  • README.md updated (if relevant)
  •  CHANGELOG.md entry added
    In my previous PR I promised to fix some tests. I made a start. I noticed in test_chagelog.py some tests fail due to different whitespaces on different machines/screens. I implemented a possible solution here, that could be made a bit more generic off course. But I was looking for your thoughts on it beforehand.

@jochenchrist jochenchrist requested a review from Copilot August 3, 2025 15:06
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR focuses on fixing various test issues and improving code compatibility. The changes address whitespace handling in test assertions, file path normalization, debugging output, and modernizing deprecated Pydantic methods.

  • Implements a whitespace-agnostic solution for test assertions in changelog tests
  • Updates deprecated Pydantic dict() method to model_dump()
  • Fixes attribute access patterns using type(object).model_fields instead of object.model_fields

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/test_test_delta.py Normalizes file path with explicit relative path prefix
tests/test_import_protobuf.py Adds debugging output for YAML comparison troubleshooting
tests/test_changelog.py Implements whitespace-insensitive string comparison for test assertions
datacontract/lint/resolve.py Fixes model field access pattern using type() method
datacontract/export/rdf_converter.py Updates multiple model field access patterns to use type() method
datacontract/export/protobuf_converter.py Replaces deprecated Pydantic dict() method with model_dump()
README.md Adds Docker requirement note for test execution

@jochenchrist
Copy link
Contributor

I am not a big fan of the changes, as they make the code more difficult to read and understand.

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.

2 participants