LCORE-2400: Test handling empty values by tisnik · Pull Request #1852 · lightspeed-core/lightspeed-stack

tisnik · 2026-06-05T06:54:10Z

Description

LCORE-2400: Test handling empty values

Type of change

Tools used to create PR

Assisted-by: N/A
Generated by: N/A

Related Tickets & Documents

Related Issue #LCORE-2400

Summary by CodeRabbit

Tests
- Expanded test coverage for schema transformation edge cases, including handling of empty containers, nullable conversions, and validation of schema properties.

coderabbitai · 2026-06-05T06:54:22Z

Warning

Review limit reached

@tisnik, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 30 minutes and 15 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e9a60eb7-692c-4429-9485-b2f39add4450

📥 Commits

Reviewing files that changed from the base of the PR and between d73dd19 and b585eb8.

📒 Files selected for processing (1)

tests/unit/utils/test_schema_dumper.py

Walkthrough

This PR adds five new unit tests to test_schema_dumper.py that expand coverage for the recursive_update transformation function. The tests validate edge cases including preservation of object schema metadata, empty collection handling, and correct anyOf-to-nullable conversion when additional schema constraints are present.

Changes

Extended recursive_update Test Coverage

Layer / File(s)	Summary
Extended recursive_update test cases `tests/unit/utils/test_schema_dumper.py`	Five new test functions validate that `recursive_update` preserves non-targeted object keys (e.g., `required`) while converting nested `anyOf` with `null` to `nullable` form, leaves empty containers unchanged, and performs anyOf-to-nullable transformation correctly when extra schema fields like `format`, `maxLength`, `exclusiveMinimum`, and `description` are present.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

lightspeed-core/lightspeed-stack#1834: Both PRs add unit tests in tests/unit/utils/test_schema_dumper.py covering recursive_update transformations for anyOf with null becoming nullable (and this PR also extends coverage with additional schema fields).
lightspeed-core/lightspeed-stack#1827: This PR expands tests/unit/utils/test_schema_dumper.py with additional recursive_update unit tests that build directly on the same recursive_update test suite added in that PR, focusing on anyOf→nullable and required/properties preservation.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title mentions testing empty values, which aligns with two of five new tests (test_handles_empty_lists and test_handles_empty_maps), but omits the broader context of also testing recursive_update behavior, anyOf-to-nullable conversion, and property preservation.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

✨ Simplify code

Create PR with simplified code

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with 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.

Inline comments:
In `@tests/unit/utils/test_schema_dumper.py`:
- Around line 370-405: The tests test_anyof_with_additional_fields_on_first_item
and test_anyof_with_additional_fields_more_items assert expected schemas that
drop branch-specific constraints (like "format" and "maxLength") when converting
an anyOf with a null branch; update the expected dictionaries in those tests so
they preserve the non-null branch's constraints (e.g., include "format": "email"
and "maxLength": 50) while still converting type to "string" and adding
"nullable": True (and keep the existing mapping of "exclusiveMinimum" ->
"minimum" in test_anyof_with_additional_fields_more_items); this ensures
recursive_update's anyOf->nullable transformation retains sibling constraints
from the chosen branch.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 51e52205-b503-46dc-88b8-c0375592fb69

📥 Commits

Reviewing files that changed from the base of the PR and between 71cd53c and d73dd19.

📒 Files selected for processing (1)

tests/unit/utils/test_schema_dumper.py

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)

GitHub Check: integration_tests (3.13)
GitHub Check: integration_tests (3.12)
GitHub Check: spectral
GitHub Check: unit_tests (3.13)
GitHub Check: unit_tests (3.12)
GitHub Check: mypy
GitHub Check: build-pr
GitHub Check: Pylinter
GitHub Check: E2E: library mode / ci / group 1
GitHub Check: E2E: library mode / ci / group 2
GitHub Check: E2E: server mode / ci / group 1
GitHub Check: E2E: server mode / ci / group 2
GitHub Check: E2E: server mode / ci / group 3
GitHub Check: E2E: library mode / ci / group 3
GitHub Check: E2E Tests for Lightspeed Evaluation job
GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request

🧰 Additional context used

📓 Path-based instructions (1)

tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

tests/**/*.py: Use pytest for all unit and integration tests; do not use unittest
Use pytest.mark.asyncio marker for async tests

Files:

tests/unit/utils/test_schema_dumper.py

🔇 Additional comments (1)

tests/unit/utils/test_schema_dumper.py (1)

309-333: LGTM!

Also applies to: 346-368

coderabbitai Bot reviewed Jun 5, 2026

View reviewed changes

Comment thread tests/unit/utils/test_schema_dumper.py

tisnik force-pushed the lcore-2400-test-handling-empty-values branch 2 times, most recently from c69b912 to b1f5e9c Compare June 5, 2026 07:12

LCORE-2400: Test handling empty values

b585eb8

tisnik force-pushed the lcore-2400-test-handling-empty-values branch from b1f5e9c to b585eb8 Compare June 5, 2026 07:23

tisnik merged commit 014a5ba into lightspeed-core:main Jun 5, 2026
40 of 42 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

LCORE-2400: Test handling empty values#1852

LCORE-2400: Test handling empty values#1852
tisnik merged 1 commit into
lightspeed-core:mainfrom
tisnik:lcore-2400-test-handling-empty-values

tisnik commented Jun 5, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 5, 2026 •

edited

Loading

Review limit reached

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tisnik commented Jun 5, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Tools used to create PR

Related Tickets & Documents

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tisnik commented Jun 5, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 5, 2026 •

edited

Loading