Comfy-Org / Comfy-Org/ComfyUI_frontend
Re-enable Skipped Litegraph Tests After Migration
- 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
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