mirror of
https://github.com/Comfy-Org/ComfyUI-Manager.git
synced 2025-12-16 18:02:58 +08:00
test: fix enable/disable API parameter mismatch across all tests
Fix critical parameter naming issue causing 50% test failure rate.
All enable/disable operations were using incorrect parameter names,
causing silent failures (200 OK but no state change).
Root Cause:
- Disable operations require "node_name" parameter
- Enable operations require "cnr_id" parameter
- All tests were incorrectly using "id" parameter
- Queue/task endpoint validates params strictly via Pydantic models
- Invalid params cause silent failures with 200 OK response
Changes Applied:
- 14 disable operations: {"id": ...} → {"node_name": ...}
- 7 enable operations: {"id": ...} → {"cnr_id": ...}
- Added tests/check_test_results.sh for clean result monitoring
Files Modified:
- tests/glob/conftest.py (4 disable fixes)
- tests/glob/test_installed_api_enabled_priority.py (3 disable + 1 enable)
- tests/glob/test_enable_disable_api.py (6 disable + 4 enable)
- tests/glob/test_complex_scenarios.py (1 disable + 2 enable)
- tests/check_test_results.sh (new utility script)
Test Results:
- Before fixes: 5/10 environments passing (50%)
- After fixes: 9/10 environments passing (90%)
- Improvement: +4 environments (+80%)
Session Progress:
- Session start: 7/10 environments
- Current state: 9/10 environments
- Total improvement: +2 environments (+28.6%)
Remaining Work:
- 1 failure in Env 9: test_installed_api_no_duplicates_across_scenarios
- Issue: Package showing enabled=False in "CNR enabled + Nightly disabled" scenario
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
2b778fd42c
commit
fb3a67f22c
35
tests/check_test_results.sh
Executable file
35
tests/check_test_results.sh
Executable file
@ -0,0 +1,35 @@
|
|||||||
|
#!/bin/bash
|
||||||
|
# Simple test result checker
|
||||||
|
# Usage: ./tests/check_test_results.sh [logfile]
|
||||||
|
|
||||||
|
LOGFILE=${1:-/tmp/test-param-fix-final.log}
|
||||||
|
|
||||||
|
if [ ! -f "$LOGFILE" ]; then
|
||||||
|
echo "Log file not found: $LOGFILE"
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# Check if tests are complete
|
||||||
|
if grep -q "Test Results Summary" "$LOGFILE"; then
|
||||||
|
echo "========================================="
|
||||||
|
echo "Test Results"
|
||||||
|
echo "========================================="
|
||||||
|
echo ""
|
||||||
|
|
||||||
|
# Show summary
|
||||||
|
grep -A 30 "Test Results Summary" "$LOGFILE" | head -40
|
||||||
|
|
||||||
|
echo ""
|
||||||
|
echo "========================================="
|
||||||
|
|
||||||
|
# Count passed/failed
|
||||||
|
PASSED=$(grep -c "✅.*PASSED" "$LOGFILE")
|
||||||
|
FAILED=$(grep -c "❌.*FAILED" "$LOGFILE")
|
||||||
|
|
||||||
|
echo "Environments: Passed=$PASSED, Failed=$FAILED"
|
||||||
|
|
||||||
|
else
|
||||||
|
echo "Tests still running..."
|
||||||
|
echo "Last 10 lines:"
|
||||||
|
tail -10 "$LOGFILE"
|
||||||
|
fi
|
||||||
@ -204,7 +204,7 @@ def test_installed_api_shows_disabled_when_no_enabled_exists(
|
|||||||
response = api_client.queue_task(
|
response = api_client.queue_task(
|
||||||
kind="disable",
|
kind="disable",
|
||||||
ui_id="test_disabled_only_disable",
|
ui_id="test_disabled_only_disable",
|
||||||
params={"id": TEST_PACKAGE_ID},
|
params={"node_name": TEST_PACKAGE_ID},
|
||||||
)
|
)
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
|
|
||||||
@ -246,6 +246,17 @@ def test_installed_api_shows_disabled_when_no_enabled_exists(
|
|||||||
f"Package should be disabled, got: {package_info}"
|
f"Package should be disabled, got: {package_info}"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Cleanup: Re-enable package for other tests
|
||||||
|
response = api_client.queue_task(
|
||||||
|
kind="enable",
|
||||||
|
ui_id="test_disabled_only_cleanup",
|
||||||
|
params={"cnr_id": TEST_PACKAGE_ID},
|
||||||
|
)
|
||||||
|
assert response.status_code == 200
|
||||||
|
response = api_client.start_queue()
|
||||||
|
assert response.status_code in [200, 201]
|
||||||
|
time.sleep(WAIT_TIME_SHORT)
|
||||||
|
|
||||||
|
|
||||||
def test_installed_api_no_duplicates_across_scenarios(
|
def test_installed_api_no_duplicates_across_scenarios(
|
||||||
api_client,
|
api_client,
|
||||||
@ -306,7 +317,7 @@ def test_installed_api_no_duplicates_across_scenarios(
|
|||||||
response = api_client.queue_task(
|
response = api_client.queue_task(
|
||||||
kind="disable",
|
kind="disable",
|
||||||
ui_id=f"test_{scenario_id}_disable",
|
ui_id=f"test_{scenario_id}_disable",
|
||||||
params={"id": TEST_PACKAGE_ID},
|
params={"node_name": TEST_PACKAGE_ID},
|
||||||
)
|
)
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
response = api_client.start_queue()
|
response = api_client.start_queue()
|
||||||
@ -415,7 +426,7 @@ def test_installed_api_cnr_priority_when_both_disabled(
|
|||||||
response = api_client.queue_task(
|
response = api_client.queue_task(
|
||||||
kind="disable",
|
kind="disable",
|
||||||
ui_id="test_cnr_priority_nightly_disable",
|
ui_id="test_cnr_priority_nightly_disable",
|
||||||
params={"id": TEST_PACKAGE_ID},
|
params={"node_name": TEST_PACKAGE_ID},
|
||||||
)
|
)
|
||||||
assert response.status_code == 200
|
assert response.status_code == 200
|
||||||
response = api_client.start_queue()
|
response = api_client.start_queue()
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user