Skip to content

Commit 1d6cd86

Browse files
Add markers and instructions to extract branches from long functions
1 parent 7154d08 commit 1d6cd86

File tree

4 files changed

+67
-1
lines changed

4 files changed

+67
-1
lines changed

.github/CONTRIBUTING.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Contributing
22

3+
### Testing
4+
5+
See `tests/README.md` for guidance on the scaffold-based testing pattern used in this project.
6+
37
### Code Style
48

59
This project uses [`black`](https://github.com/psf/black) to automatically

AGENTS.md

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,4 +43,13 @@ Best practice:
4343
* Pydantic or LinkML is favored for data objects
4444
* For state in engine-style OO classes, dataclasses is favored
4545
* Declarative principles are favored
46-
* Always use type hints, always document methods and classes
46+
* Always use type hints, always document methods and classes
47+
* **Always add tests when implementing new features or fixing bugs.** Don't wait to be asked.
48+
* For transformation tests, use the scaffold-based testing pattern described in `tests/README.md`
49+
50+
## Code Patterns
51+
52+
When modifying `ObjectTransformer.map_object`, check for `# EXTRACT:` markers.
53+
If touching a marked section, extract it to a private method first and add a test
54+
in `tests/test_transformer/test_object_transformer_new.py` using `@add_to_test_setup`.
55+
See issue #104 for context.

src/linkml_map/transformer/object_transformer.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,13 @@ def index(self, source_obj: Any, target: Optional[str] = None) -> None:
145145
else:
146146
self.object_index = ObjectIndex(source_obj, schemaview=self.source_schemaview)
147147

148+
# Developer Note:
149+
# This method has grown large. When modifying it, consider extracting to
150+
# private methods and adding tests using the scaffold-based testing pattern.
151+
# See EXTRACT markers below for candidates.
152+
#
153+
# See: tests/README.md for testing guidance
154+
# Tracking: https://github.com/linkml/linkml-map/issues/104
148155
def map_object(
149156
self,
150157
source_obj: OBJECT_TYPE,
@@ -161,6 +168,7 @@ def map_object(
161168
:return: transformed data, either as type target_type or a dictionary
162169
"""
163170
sv = self.source_schemaview
171+
# EXTRACT: _resolve_source_type(sv, source_obj) -> str
164172
if source_type is None and sv is None:
165173
# TODO: use smarter method
166174
source_type = next(iter(self.specification.class_derivations.values())).name
@@ -227,6 +235,7 @@ def map_object(
227235
v = self._perform_melt(
228236
slot_derivation.pivot_operation, source_obj, slot_derivation
229237
)
238+
# EXTRACT: _derive_from_expr(slot_derivation, source_obj, source_obj_typed, source_type, sv) -> Any
230239
elif slot_derivation.expr:
231240
if bindings is None:
232241
bindings = Bindings(
@@ -248,6 +257,7 @@ def map_object(
248257
aeval = Interpreter(usersyms={"src": ctxt_obj, "target": None})
249258
aeval(slot_derivation.expr)
250259
v = aeval.symtable["target"]
260+
# EXTRACT: _derive_from_populated_from(slot_derivation, source_obj, sv, source_type) -> tuple[Any, SlotDefinition]
251261
elif slot_derivation.populated_from:
252262
populated_from = slot_derivation.populated_from
253263
fk_resolution = resolve_fk_path(sv, source_type, populated_from)
@@ -309,6 +319,7 @@ def map_object(
309319
logger.debug(
310320
f"Pop slot {slot_derivation.name} => {v} using {slot_derivation.populated_from} // {source_obj}"
311321
)
322+
# EXTRACT: _derive_from_object_derivations(slot_derivation, source_obj, source_type, target_type) -> Any
312323
elif slot_derivation.object_derivations:
313324
# We'll collect all derived objects here
314325
derived_objs = []
@@ -337,6 +348,7 @@ def map_object(
337348
else:
338349
source_class_slot = sv.induced_slot(slot_derivation.name, source_type)
339350
v = source_obj.get(slot_derivation.name, None)
351+
# EXTRACT: _post_process_slot_value(v, source_class_slot, slot_derivation, class_deriv) -> Any
340352
if source_class_slot and v is not None:
341353
# slot is mapped and there is a value in the assignment
342354
target_range = slot_derivation.range

tests/README.md

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
# Testing
2+
3+
## Scaffold-Based Testing Pattern
4+
5+
This project uses a **scaffold-based testing pattern** for transformation tests. The pattern:
6+
7+
1. A **scaffold** provides base schemas, transform spec, input data, and expected output
8+
2. **Setup functions** patch the scaffold incrementally using `apply_schema_patch()` / `apply_transform_patch()`
9+
3. **Unit tests** run each setup function in isolation
10+
4. **Integration tests** run all setups cumulatively, catching interaction bugs
11+
12+
This pattern is preferred for transformation tests because it:
13+
- Makes tests concise (just patches, not full schemas)
14+
- Catches both individual feature bugs and feature interaction bugs
15+
- Builds up a realistic, complex test scenario over time
16+
17+
### Existing Scaffolds
18+
19+
- **Simple** (`tests/scaffold/`): Basic slot derivations, Person→Agent transforms.
20+
Fixtures: `scaffold`, `integration_scaffold`. Decorator: `@add_to_test_setup`
21+
22+
- **Container** (`tests/scaffold_container/`): FK lookups, flattening, cross-class transforms.
23+
Fixtures: `container_scaffold`, `integration_container_scaffold`. Decorator: `@add_to_container_test_setup`
24+
25+
### When to Create a New Scaffold
26+
27+
If testing a feature that doesn't fit existing scaffolds (e.g., different schema shape,
28+
inheritance hierarchies, enum-heavy transforms), create a new scaffold directory following
29+
the same pattern. See `tests/scaffold/` for the structure.
30+
31+
### Utilities
32+
33+
See `tests/scaffold/utils/apply_patch.py` for patching utilities.
34+
35+
### When to Use Traditional Tests
36+
37+
It's okay to use traditional tests (not scaffold-based) when:
38+
- Testing utility functions (unit conversion, eval_utils, etc.)
39+
- Testing error handling or edge cases that don't fit the scaffold pattern
40+
- Testing components outside the transformer (CLI, schema mapping, inverter)
41+
- Quick regression tests for specific bugs

0 commit comments

Comments
 (0)