Adds new testing Framework with example#90
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new testing framework for LinkML-Map designed to improve test organization and reduce the "kitchen sink testing problem" by providing a scaffold-based approach with reusable test data and helper functions.
- Introduces a scaffold system with predefined schemas, transform specifications, and expected outputs
- Adds decorator-based integration test setup to compose multiple test scenarios
- Provides example tests demonstrating the framework with Person-to-Agent transformations
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_transformer/test_object_transformer_new.py | Main test file demonstrating the new framework with scaffold fixtures and integration decorators |
| tests/scaffold/init.py | Core scaffold module providing data loading and deep copy functionality for test isolation |
| tests/conftest.py | Integration test decorator and setup mechanism for composable test scenarios |
| tests/scaffold/source/person.yaml | Source schema defining Person class with id and name slots |
| tests/scaffold/target/agent.yaml | Target schema defining Agent class with id and label slots |
| tests/scaffold/transform/person_to_agent.transform.yaml | Transform specification mapping Person to Agent with slot derivations |
| tests/scaffold/data/Person-001.yaml | Sample input data for testing transformations |
| tests/scaffold/expected/Person-001.transformed.yaml | Expected output data for validation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
tests/scaffold/__init__.py
Outdated
| def yaml_load(file_path: Path) -> dict: | ||
| return yaml.safe_load(file_path.read_text()) | ||
|
|
||
| LOADED_DATA = { | ||
| "source_schema": SchemaView(SOURCE_SCHEMA), | ||
| "target_schema": SchemaView(TARGET_SCHEMA), | ||
| "transform_spec": yaml_load(TRANSFORM_SPEC), | ||
| "input_data": yaml_load(INPUT_DATA), | ||
| "expected": yaml_load(EXPECTED_DATA), | ||
| } |
There was a problem hiding this comment.
you can turn all of these bits of LOADED_DATA into fixtures (or a single fixture) -- fixtures can be reused, which can save time and computational power if you're using the same ones over and over again.
There was a problem hiding this comment.
This is where I have a bit of a knowledge gap with pytest. If I create these as fixtures and then mutate them, does it mutate the fixture globally or generate a new copy each time? Regardless, I'm creating this once and then handing off a deep-copy each time (see the next two lines); I don't access this as this data structure. I think this is functionally the same as the fixtures and it is a single structure so we don't need to dedicate a lot of boilerplate to loading all of these in each test.
There was a problem hiding this comment.
Okay, looking a little deeper. I can see that pytest does create the feature for each test, so there wouldn't be any contamination. However, I'm not sure that is any more clear than what I've done here.
There was a problem hiding this comment.
Since I'm calling the scaffold as a fixture in the test file, I don't see any value in fixture-izing these. At worst, it re-loads the data each time, when we can do it once, or encourages people to muck with the data when we should deepcopy and use the new_scaffold() function and scaffold fixture.
There was a problem hiding this comment.
I find it far easier to put all data structures and functions to produce data structures (particularly anything used by several different tests) in as fixtures so that I can let pytest take care of them -- generating them on demand, managing their life cycle, etc. I can understand that you are not familiar with fixtures ATM and so are a bit wary of using them, but they really do make life easier and the more you use them, the more you'll see how useful and flexible they can be.
There was a problem hiding this comment.
Hmm, Let me give that a try. I'll try to get some changes pushed up in the next couple of days.
|
Some general comments. - I understand this PR as an experiment towards a "perfect" test framework. The added scaffold system partially recreates what pytest fixtures already offer which features you do not use in depth. For example, Regarding the need to mutate the data of a fixture: Maybe a factory (supplied by a fixture) may be a good alternative? Also you can parametrize fixtures. For separating / marking integration tests, it would be best to separate integration from unit tests by directory - but this may be a too big change. However, pytest offers a mark-feature (which you seem to copy with The pytest.mark feature would work as follows. In pyproject.ini add Mark tests: import pytest
@pytest.mark.integration
def test_math(external_service):
"""Integration test - uses external service"""
assert external_service()Run tests based on their markers: |
|
But this isn't what I'm trying to do with this mark. Instead, I'm collecting (marking) all of the changes that we make in our separate unit tests (not the tests themselves) and applying all of those changes to the scaffold after I've collected all of the unit test transformations as a pseudo integration test. I'm not trying to mark the unit tests and integration tests for running them separately. |
|
As I mentioned in-line above. I'll try to move this a little more to the fixtures framework. |
|
Thanks to both of you for pushing back on this... and I apologize for my being a little irritated by it (I hope it didn't show). I think you're right about moving this into just base pytest fixture land. I'll have those changes soon. |
|
Thanks for your comments on this PR and the ideas for the new framework. It is a much better framework for your having pushed me to use |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Okay, I think I've addressed the comments earlier about making these fixtures. I've also expanded on that and now made things easier to read; at least I think they are. I'm going to merge it now and start trying to use it. Since this is still experimental and only a testing framework, I don't think this is high-risk. |
This is a model of the new testing framework I would like to consider for LinkML-Map. It doesn't touch any of the existing framework except to bring over a couple of tests as examples of how the framework should be used. I hope this new testing regime will help us to avoid the kitchen sink testing problem we're currently already experiencing and that has been troublesome in other repos.