Skip to content

LCORE-4898: Assertion splitted into simpler parts - #2966

Open
tisnik wants to merge 1 commit into
lightspeed-core:mainfrom
tisnik:lcore-4898
Open

tisnik wants to merge 1 commit into
lightspeed-core:mainfrom
tisnik:lcore-4898

Conversation

@tisnik

@tisnik tisnik commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Description

LCORE-4898: Assertion splitted into simpler parts

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library [pyproject.toml + uv.lock]
  • Bump-up dependent library [requirements.*.txt for Konflux]
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

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

Related Tickets & Documents

  • Related Issue #LCORE-4898

Summary by CodeRabbit

  • Tests
    • OpenAPI integration checks now distinguish between a missing or invalid server list and a valid but empty list, with clearer failure messages for each condition.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lightspeed-core/lightspeed-stack/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: ef0af675-744b-40da-a231-b853e780fe1e
📥 Commits

Reviewing files that changed from the base of the PR and between 9794767 and 6df05e0.

📒 Files selected for processing: 1
  • tests/integration/test_openapi_json.py
📜 Recent review details
⏰ Context from checks skipped due to timeout: 37
  • GitHub Check: E2E: library / ci / default
  • GitHub Check: E2E: library / ci / other
  • GitHub Check: E2E: library / ci / shields
  • GitHub Check: E2E: library / ci / mcp
  • GitHub Check: E2E: library / ci / skills
  • GitHub Check: E2E: library / ci / rbac
  • GitHub Check: E2E: library / ci / authorized
  • GitHub Check: E2E: server / ci / shields
  • GitHub Check: E2E: server / ci / mcp
  • GitHub Check: E2E: server / ci / skills
  • GitHub Check: E2E: server / ci / rbac
  • GitHub Check: E2E: server / ci / tls
  • GitHub Check: E2E: server / ci / other
  • GitHub Check: E2E: server / ci / default
  • GitHub Check: E2E: server / ci / authorized
  • GitHub Check: Red Hat Konflux / lightspeed-stack-0-8-e2e-tests / lightspeed-stack-0-8
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: Pylinter
  • GitHub Check: unit_tests (3.14)
  • GitHub Check: check_dependencies
  • GitHub Check: radon
  • GitHub Check: black
  • GitHub Check: integration_tests (3.12)
  • GitHub Check: integration_tests (3.13)
  • GitHub Check: mypy
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: Pyright
  • GitHub Check: spectral
  • GitHub Check: integration_tests (3.14)
  • GitHub Check: build-pr
  • GitHub Check: Red Hat Konflux / lightspeed-core-0-8-enterprise-contract / lightspeed-stack-0-8
  • GitHub Check: list_outdated_dependencies
  • GitHub Check: bandit
  • GitHub Check: shellcheck
  • GitHub Check: Red Hat Konflux / rag-content-0-8-e2e-tests / lightspeed-stack-0-8
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-8-on-pull-request
  • GitHub Check: check
🔇 Additional comments: 1
tests/integration/test_openapi_json.py: 1

147-148: LGTM!

Walkthrough

The OpenAPI integration test now checks separately whether servers is a list and whether it is non-empty. Each condition has its own failure message.

Changes

OpenAPI server check

Layer / File(s) Summary
Separate server list assertions
tests/integration/test_openapi_json.py
The test now reports distinct assertion failures when servers is not a list or is empty.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Refactor

Suggested reviewers: jdubrick

Merge Risk: ⚪ Minimal · up to 6df05

This test-only change distinguishes an invalid servers type from an empty list. No material merge-blocking risk is identified.

Pre-merge checks: Passed 7
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the main change: splitting one assertion into simpler assertions. It is concise and related to the modified test.
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
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.
Performance And Algorithmic Complexity Passed PASS. The PR only splits one assertion in tests/integration/test_openapi_json.py:147-148. Both checks are constant-time operations on the existing servers value. The change adds no loops, API call…
Security And Secret Handling Passed PASSED. The pull request changes only assertion structure and messages in tests/integration/test_openapi_json.py (lines 147–148). It introduces no secrets, authentication changes, injection paths, s…
Full details: Performance And Algorithmic Complexity

Explanation

PASS. The PR only splits one assertion in tests/integration/test_openapi_json.py:147-148. Both checks are constant-time operations on the existing servers value. The change adds no loops, API calls, parsing, caching, buffering, or list operations with unbounded growth.

Full details: Security And Secret Handling

Explanation

PASSED. The pull request changes only assertion structure and messages in tests/integration/test_openapi_json.py (lines 147–148). It introduces no secrets, authentication changes, injection paths, sensitive output, or Kubernetes Secret configuration.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
✨ Simplify code
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@tisnik

tisnik commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

/retest

@red-hat-konflux-kflux-prd-rh02

Copy link
Copy Markdown
Contributor

All PipelineRuns for this commit have already succeeded. Use /retest <pipeline-name> to re-run a specific pipeline or /test to re-run all pipelines.

@tisnik

tisnik commented Oct 19, 2026

Copy link
Copy Markdown
Contributor Author

/retest Red Hat Konflux / lightspeed-stack-0-8-e2e-tests / lightspeed-stack-0-8

@openshift-ci

openshift-ci Bot commented Oct 19, 2026

Copy link
Copy Markdown

@tisnik: No presubmit jobs available for lightspeed-core/lightspeed-stack@main

Details

In response to this:

/retest Red Hat Konflux / lightspeed-stack-0-8-e2e-tests / lightspeed-stack-0-8

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

This branch has not been deployed

No deployments
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.

1 participant