Skip to content

LCORE-2400: Unit tests for schema dumper #1827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tisnik merged 1 commit into from
Jun 1, 2026
Merged

LCORE-2400: Unit tests for schema dumper #1827

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions tests/unit/utils/test_schema_dumper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
"""Unit tests for utils/schema_dumper module."""

from typing import Any

from utils.schema_dumper import recursive_update


def test_update_empty_input() -> None:
"""Test how recursive_update function transforms empty input."""
original: dict[str, Any] = {}
expected: dict[str, Any] = {}

# perform the update
result = recursive_update(original)

# empty dict should be returned
assert result == expected

# ensure a new dict is returned, not the same object
assert result is not original


def test_no_change_for_simple_schema() -> None:
"""Test how recursive_update function trasforms simple non-empty input."""
original: dict[str, Any] = {
"type": "string",
"maxLength": 10,
}

# we need to distinguish between original and a copy
expected = original.copy()

# perform the update
result = recursive_update(original)

# non-empty dict with known content should be returned
assert result == expected

# ensure a new dict is returned, not the same object
assert result is not original


def test_no_change_for_simple_object() -> None:
"""Test how recursive_update function trasforms simple non-empty input."""
original: dict[str, Any] = {
"type": "object",
"properties": {
"name": {"type": "string"},
"age": {"type": "integer"},
},
}

# we need to distinguish between original and a copy
expected = original.copy()

@coderabbitai coderabbitai Bot Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Use deep copy for nested schema expectations.

original.copy() is shallow here, so nested mutations can be missed. Use a deep copy to make this test reliably detect unintended in-place nested changes.

Proposed fix 
+from copy import deepcopy
@@
-    expected = original.copy()
+    expected = deepcopy(original)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
expected = original.copy()
expected = deepcopy(original)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/unit/utils/test_schema_dumper.py` at line 55, The test currently uses a
shallow copy via original.copy() which won't protect against nested mutations;
replace that with a deep copy so nested structures are duplicated (e.g., use
copy.deepcopy(original)) and add the required import for the copy module at the
top of the test file; update the variable assignment for expected to use the
deep copy so the test reliably detects in-place nested changes.

@tisnik tisnik Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@coderabbitai create issue

@coderabbitai coderabbitai Bot Jun 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
gh issue create \
  --repo lightspeed-core/lightspeed-stack \
  --title "Use deep copy for nested schema expectations in test_schema_dumper.py" \
  --assignee tisnik \
  --body "## Summary

In \`tests/unit/utils/test_schema_dumper.py\`, the test \`test_no_change_for_simple_object\` (and potentially other tests) uses \`original.copy()\` (a shallow copy) to create the \`expected\` dict. Since the schema dicts contain nested structures (e.g., \`properties\`), a shallow copy will not protect against unintended in-place nested mutations, making the test unreliable for detecting such bugs.

## Suggested Fix

Replace \`original.copy()\` with \`copy.deepcopy(original)\` and add the required import at the top of the file:

\`\`\`python
from copy import deepcopy
\`\`\`

Then update the assignment:

\`\`\`python
expected = deepcopy(original)
\`\`\`

This change should be applied wherever \`original.copy()\` is used for nested dicts in the test file.

## References

- PR: https://github.com/lightspeed-core/lightspeed-stack/pull/1827
- Review comment: https://github.com/lightspeed-core/lightspeed-stack/pull/1827#discussion_r3332405313

/cc `@tisnik`"

Length of output: 1274

@tisnik, the issue has been created and assigned to you: #1828 🐇


# perform the update
result = recursive_update(original)

# non-empty dict with known content should be returned
assert result == expected

# ensure a new dict is returned, not the same object
assert result is not original


def test_recursive_recurse_into_subdicts() -> None:
"""Test the recursive_update on input containing sub-dictionaries."""
original = {
"type": "object",
"properties": {
"name": {"type": "string"},
"age": {"type": "integer", "exclusiveMinimum": 0},
},
}
expected = {
"type": "object",
"properties": {
"name": {"type": "string"},
"age": {"type": "integer", "minimum": 0},
},
}

# perform the update
result = recursive_update(original)

# non-empty dict with known content should be returned
assert result == expected

# ensure a new dict is returned, not the same object
assert result is not original


def test_handles_none_values() -> None:
"""None values should be preserved."""
original = {"key": None}
expected = original.copy()

# perform the update
result = recursive_update(original)

# non-empty dict with known content should be returned
assert result == expected
Loading