Comfy-Org / Comfy-Org/ComfyUI_frontend

Re-enable Skipped Litegraph Tests After Migration

Open
#5,081 0 comments 0 reactions 0 assignees View on GitHub
area:testing
Dominant language
TypeScript
Stars
2k
Forks
704
Avg merge
1d 8h
Merged PRs (30d)
512

Description

## Overview
During the litegraph test migration (PR #5063), 175 tests across 18 files were disabled using `test.skip` due to compatibility issues. These tests need to be fixed and re-enabled.

## Affected Test Categories

### 1. **Subgraph Tests** (14 files, ~140 skipped tests)
Most heavily affected area with issues in:
- Event handling and cleanup
- Widget promotion
- Serialization/deserialization
- Memory management
- I/O slot connections
- Nested subgraph execution

### 2. **Canvas Tests** (2 files, ~8 skipped tests)
- `LinkConnectorSubgraphInputValidation.test.ts`
- `LinkConnector.test.ts`

### 3. **Core Tests** (2 files, ~2 skipped tests)
- `ConfigureGraph.test.ts`
- `LGraph_constructor.test.ts`

## Root Causes

### Type Definition Mismatches
- 85 `@ts-expect-error` comments indicate missing properties/methods
- Common missing properties: `visible`, `color`, `drawContent`
- Constructor argument type issues
- Null/undefined handling differences

### Possible Structural Changes
- Tests expect functionality that may have been refactored
- Import path changes from direct imports to barrel imports
- Potential API changes in litegraph library

## Proposed Solutions

1. **Audit Type Definitions**
- Compare test expectations with current litegraph implementation
- Update type definitions to include missing properties
- Fix constructor signatures

2. **Update Test Fixtures**
- Review and update test setup/teardown
- Ensure fixtures match current API

3. **Incremental Approach**
- Start with core tests (smallest scope)
- Move to canvas tests
- Finally tackle subgraph tests (largest scope)

4. **Consider Test Validity**
- Some tests may be testing deprecated functionality
- Evaluate if all tests are still relevant

## Next Steps
1. Create individual PRs for each test category
2. Document any API changes discovered during fixes
3. Update test documentation as needed

## Related
- Original migration PR: #5063
- Test location: `/tests-ui/tests/litegraph/`

┆Issue is synchronized with this [Notion page](https://www.notion.so/Issue-5081-Re-enable-Skipped-Litegraph-Tests-After-Migration-2536d73d365081a28b4fe97520e85fae) by [Unito](https://www.unito.io)

Contributor guide

Open the contributing guide

Research direction

Start in /tests-ui/tests/litegraph/ with ConfigureGraph.test.ts and LGraph_constructor.test.ts, as proposed, and compare their failures with the current litegraph type definitions and API. Then work through the canvas and subgraph test files, checking fixtures and deprecated expectations. Done means the 175 skipped tests across 18 files are re-enabled and passing, or obsolete cases and API changes are documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend, testing
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.