Replies: 3 comments
-
|
Absolutely, Roger — here is everything you asked for, clean, precise, and ready to paste into a PR right now:
This patch updates the CI so that test reductions are allowed when a TEST CHANGE JUSTIFICATION is present, exactly matching your architectural shift. ✅ 1. PR‑READY PATCH (unified diff)Paste this directly into a GitHub PR as a patch file or apply via diff --git a/.github/workflows/test-integrity.yml b/.github/workflows/test-integrity.yml
index abc123..def456 100644
--- a/.github/workflows/test-integrity.yml
+++ b/.github/workflows/test-integrity.yml
@@ -75,14 +75,36 @@ jobs:
- name: Enforce test integrity
run: |
- if [ "$PR_TEST_FILES" -lt "$BASELINE_TEST_FILES" ]; then
- echo "❌ Test file count decreased"
- exit 1
- fi
-
- if [ "$PR_ASSERTIONS" -lt "$BASELINE_ASSERTIONS" ]; then
- echo "❌ Assertion count decreased"
- exit 1
- fi
+ body="${{ github.event.pull_request.body }}"
+
+ # -----------------------------
+ # Test file count check
+ # -----------------------------
+ if [ "$PR_TEST_FILES" -lt "$BASELINE_TEST_FILES" ]; then
+ if ! grep -qi "TEST CHANGE JUSTIFICATION" <<< "$body"; then
+ echo "❌ Test file count decreased"
+ exit 1
+ else
+ echo "⚠️ Test file count decreased but justification provided — allowed"
+ fi
+ fi
+
+ # -----------------------------
+ # Assertion count check
+ # -----------------------------
+ if [ "$PR_ASSERTIONS" -lt "$BASELINE_ASSERTIONS" ]; then
+ if ! grep -qi "TEST CHANGE JUSTIFICATION" <<< "$body"; then
+ echo "❌ Assertion count decreased"
+ exit 1
+ else
+ echo "⚠️ Assertion count decreased but justification provided — allowed"
+ fi
+ fi
+
+ echo "✅ Test integrity checks passed (with justification)"This patch:
✅ 2. Updated CI YAML Block (drop‑in replacement)If your test integrity logic lives inside a larger workflow, replace the enforcement step with this: - name: Enforce test integrity
run: |
body="${{ github.event.pull_request.body }}"
if [ "$PR_TEST_FILES" -lt "$BASELINE_TEST_FILES" ]; then
if ! grep -qi "TEST CHANGE JUSTIFICATION" <<< "$body"; then
echo "❌ Test file count decreased"
exit 1
else
echo "⚠️ Test file count decreased but justification provided — allowed"
fi
fi
if [ "$PR_ASSERTIONS" -lt "$BASELINE_ASSERTIONS" ]; then
if ! grep -qi "TEST CHANGE JUSTIFICATION" <<< "$body"; then
echo "❌ Assertion count decreased"
exit 1
else
echo "⚠️ Assertion count decreased but justification provided — allowed"
fi
fi
echo "✅ Test integrity checks passed (with justification)"✅ 3. Commit Message (ready to paste)✅ 4. Full Diff (copy/paste into GitHub PR comment)Roger, this patch will immediately unblock your PR, and it aligns perfectly with your governance model and the architectural shift you’ve just completed. |
Beta Was this translation helpful? Give feedback.
-
|
Summary Changes Multi-tool refactor invalidated old test paths TEST-CHANGE: Updated all tests to use job_tools table: Removed tool= from Job creation in all tests Why the Old Tests Were Wrong Wrong scope Caused CI failures What the Pipeline Validator Now Checks Cycle detection Type compatibility Ensures input_types → output_types match across edges server/tests/app/models/test_job_multi_tool.py - Tested old schema with tool column (dropped in migration 011) Replaced by: server/tests/app/models/test_job_tool.py - Tests new job_tools table error |
Beta Was this translation helpful? Give feedback.
-
|
You didn’t do anything wrong in the PR body—the CI script is choking on it. What’s happening:
The fix is simple: stop assigning the whole PR body to a shell variable and instead pipe the GitHub expression directly into Here’s the exact YAML you should change. 1. Fix
|
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
-
Run if [ "$PR_TEST_FILES" -lt "$BASELINE_TEST_FILES" ]; then
if [ "$PR_TEST_FILES" -lt "$BASELINE_TEST_FILES" ]; then
echo "❌ Test file count decreased"
exit 1
fi
if [ "$PR_ASSERTIONS" -lt "$BASELINE_ASSERTIONS" ]; then
echo "❌ Assertion count decreased"
exit 1
fi
shell: /usr/bin/bash -e {0}
env:
BASELINE_TEST_FILES: 212
BASELINE_ASSERTIONS: 6900
PR_TEST_FILES: 209
PR_ASSERTIONS: 6700
❌ Test file count decreased
Error: Process completed with exit code 1.
this in the pr
EST CHANGE JUSTIFICATION:
Removed obsolete tests due to schema redesign
Multi-tool refactor invalidated old test paths
TEST-CHANGE: Updated all tests to use job_tools table:
Removed tool= from Job creation in all tests
Added JobTool entries for each test job
Updated assertions to query job_tools table
Deleted obsolete test_job_multi_tool.py (tested old schema)
Fixed test_job_schema_multi_tool.py to use tools field
Pipeline Test Deletions — Architectural Rationale
The deleted pipeline tests (test_yolo_ocr_pipeline.py and test_ocr_only_pipeline.py) previously attempted to validate that pipeline JSONs referenced real plugin tool IDs from the forgesyte-plugins repository.
This approach is architecturally incorrect under the split‑repo design.
Why the Old Tests Were Wrong
Wrong location
Plugin validation belongs in the forgesyte-plugins repo, not ForgeSyte Core.
Wrong scope
ForgeSyte Core is intentionally plugin‑agnostic.
Pipelines reference abstract tool types (plugin_type, tool_type), not concrete plugin IDs.
Caused CI failures
Tests failed because yolo.detect_objects does not exist.
The actual plugin is yolo-tracker, with tools such as player_detection.
Beta Was this translation helpful? Give feedback.
All reactions