Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

test: Fix openapi ingest test by reordering#1657

Draft
rjmunro wants to merge 1 commit intomainfrom
rjmunro/fix-tests
Draft

test: Fix openapi ingest test by reordering#1657
rjmunro wants to merge 1 commit intomainfrom
rjmunro/fix-tests

Conversation

Copy link
Contributor

rjmunro commented Feb 17, 2026 *
edited by coderabbitai bot
Loading

About the Contributor

Type of Contribution

This is a Bug fix

Current Behavior

main is failing CI.

New Behavior

Fix main.

Move all deletion tests to the end of the test suite to prevent cascading failures. Previously, deletion tests were interspersed throughout, causing subsequent tests to fail when trying to access deleted resources (playlists, rundowns, segments, parts).

Also add null check for newIngestPart before accessing its properties.

Testing

  • I have added one or more unit tests for this PR
  • I have updated the relevant unit tests
  • No unit test changes are needed for this PR

Affected areas

Time Frame

Other Information

Status

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

Summary

This PR reorders the OpenAPI ingest test suite to prevent test failures caused by later tests attempting to access resources that have been deleted. All deletion-related tests (for playlists, rundowns, segments, and parts) have been moved to a consolidated section at the end of the test file. Additionally, a null check has been added to guard against accessing properties on newIngestPart before it's validated.

Changes

File: packages/openapi/src/__tests__/ingest.spec.ts

  • Consolidated all deletion tests into a single "DELETIONS" block at the end of the test suite, including:

    • Playlist deletion tests (delete-by-id and bulk-delete)
    • Rundown deletion tests
    • Segment deletion tests
    • Part deletion tests

  • Added a null/undefined check for newIngestPart before accessing its properties during part update operations

  • Test semantics and expected outcomes remain unchanged; only execution order has been modified

Statistics

  • Lines changed: +52/-47
  • Single file modified
  • No changes to public APIs or method signatures

Move all deletion tests to the end of the test suite to prevent
cascading failures. Previously, deletion tests were interspersed
throughout, causing subsequent tests to fail when trying to access
deleted resources (playlists, rundowns, segments, parts).

Also add null check for newIngestPart before accessing its properties.
rjmunro requested a review from a team as a code owner February 17, 2026 14:17
Copy link

coderabbitai bot commented Feb 17, 2026 *
edited
Loading

Walkthrough

Test file refactoring that consolidates deletion-related tests into a dedicated block at the end of the test suite and adds a defensive guard check to verify object existence before mutation during part updates.

Changes

Cohort / File(s) Summary
Test Reorganization & Guards
packages/openapi/src/__tests__/ingest.spec.ts		 Moves deletion tests (parts, segments, rundowns, playlists) to consolidated "DELETIONS" block at end of suite; adds conditional guard to verify newIngestPart exists before updating its name property.

Estimated code review effort

2 (Simple) | ~10 minutes

Possibly related PRs

  • feat: Ingest API #1469: Directly modifies the same ingest.spec.ts test file with identical changes (deletion test reordering and part update guard addition).

Suggested labels

Contribution

Suggested reviewers

  • jstarpl
  • Julusian

Poem

The tests hop along in a new order line,
Deletions grouped tidily, organized so fine,
A guard now checks before the part takes a bow,
Defensive code hops in--much safer somehow!

Pre-merge checks | 3
Passed checks (3 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit's high-level summary is enabled.
Title check Passed The title 'test: Fix openapi ingest test by reordering' is concise, specific, and directly describes the main change: reordering tests to fix CI failures.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Tip: You can configure your own custom pre-merge checks in the settings.

Finishing touches
  • Generate docstrings
Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch rjmunro/fix-tests

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.

Copy link

codecov bot commented Feb 17, 2026

Codecov Report

All modified and coverable lines are covered by tests.

Thoughts on this report? Let us know!

coderabbitai bot reviewed Feb 17, 2026
Copy link

coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Nitpick comments (1)
packages/openapi/src/__tests__/ingest.spec.ts (1)

356-388: newIngestPart mutation is unused -- the putPart call uses a hardcoded literal.

The guard at lines 358-360 is a good defensive addition, but note that newIngestPart.name is mutated and then never referenced again -- the API call on line 361 passes a hardcoded part object (externalId: 'part1', name: 'Part 1', etc.), not newIngestPart. This makes the entire if block dead code. Was the intent to pass newIngestPart (or its modified name) into the putPart call?

Prompt for AI Agents Verify each finding against the current code and only fix it if needed.

In `@packages/openapi/src/__tests__/ingest.spec.ts` around lines 356 - 388, The
test contains dead code: the conditional mutates newIngestPart.name but the
ingestApi.putPart call passes a hardcoded part object, so the mutation is never
used; either remove the unused if-block or change the putPart invocation in the
'Can update a part' test to pass the modified newIngestPart (or a copy with the
updated name) as the part parameter instead of the hardcoded literal; update
references to newIngestPart and ingestApi.putPart accordingly so the mutated
name is actually sent to the API.
Prompt for all review comments with AI agents Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/openapi/src/__tests__/ingest.spec.ts`:
- Around line 356-388: The test contains dead code: the conditional mutates
newIngestPart.name but the ingestApi.putPart call passes a hardcoded part
object, so the mutation is never used; either remove the unused if-block or
change the putPart invocation in the 'Can update a part' test to pass the
modified newIngestPart (or a copy with the updated name) as the part parameter
instead of the hardcoded literal; update references to newIngestPart and
ingestApi.putPart accordingly so the mutated name is actually sent to the API.
Copy link

sonarqubecloud bot commented Feb 17, 2026

rjmunro marked this pull request as draft February 17, 2026 14:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant