fix: add missing validations/unit-tests.yaml example #1211
@@ -0,0 +1,24 @@
|
||||
# Example: Required validation for unit tests
|
||||
# Mirrors the workflow example in docs/specification.md (Example 1, Step 1).
|
||||
|
||||
name: local/unit-tests
|
||||
description: Unit tests
|
||||
source: custom
|
||||
mode: required
|
||||
code: |
|
||||
import subprocess
|
||||
|
||||
def run(input_data):
|
||||
result = subprocess.run(["pytest", "tests/"], capture_output=True, text=True)
|
||||
passed = result.returncode == 0
|
||||
return {
|
||||
"passed": passed,
|
||||
"message": "Unit tests passed" if passed else "Unit tests failed",
|
||||
"data": {
|
||||
"stdout": result.stdout,
|
||||
"stderr": result.stderr,
|
||||
"returncode": result.returncode,
|
||||
},
|
||||
}
|
||||
|
||||
timeout: 300
|
||||
@@ -1,4 +1,4 @@
|
||||
@tdd_expected_fail @tdd_issue @tdd_issue_1039
|
||||
@tdd_issue @tdd_issue_1039
|
||||
Feature: TDD Bug #1039 — Missing validations/unit-tests.yaml configuration
|
||||
As a developer
|
||||
I want to verify that the validation configuration file referenced by the
|
||||
@@ -12,14 +12,12 @@ Feature: TDD Bug #1039 — Missing validations/unit-tests.yaml configuration
|
||||
output, this validation should run ``pytest tests/``, have description
|
||||
"Unit tests", mode "required", and timeout 300s (default).
|
||||
|
||||
However, the file ``validations/unit-tests.yaml`` does not exist anywhere
|
||||
in the project. The ``examples/validations/`` directory contains only
|
||||
``required-validation.yaml`` and ``wrapped-validation.yaml``, but not
|
||||
the ``unit-tests.yaml`` referenced by the specification.
|
||||
This bug existed because no concrete ``unit-tests.yaml`` example was
|
||||
available in the repository, which made the workflow snippet difficult
|
||||
to follow and validate.
|
||||
|
||||
This test captures bug #1039 and uses ``@tdd_expected_fail`` until the
|
||||
fix is merged. Once the file is created, the tag will be removed and
|
||||
the test will run normally as a regression guard.
|
||||
This test now runs as a regression guard to ensure the example validation
|
||||
file remains present and correctly structured.
|
||||
|
||||
Scenario: The unit-tests.yaml validation config file exists in the project
|
||||
Given the project root directory is known for validation yaml test
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import json
|
||||
import os
|
||||
import shutil
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
@@ -44,7 +45,11 @@ def _pabot_parallel_args(posargs: list[str]) -> list[str]:
|
||||
)
|
||||
if has_custom_processes:
|
||||
return []
|
||||
return ["--processes", str(_default_processes())]
|
||||
# Integration tests are significantly heavier than unit tests and can
|
||||
|
|
||||
# become unstable on shared runners when pabot fans out aggressively.
|
||||
# Keep default parallelism conservative (<=2) unless explicitly overridden.
|
||||
pabot_default = min(2, _default_processes())
|
||||
return ["--processes", str(pabot_default)]
|
||||
|
||||
|
||||
def _split_pabot_args(posargs: list[str]) -> tuple[list[str], list[str]]:
|
||||
@@ -263,7 +268,9 @@ def integration_tests(session: nox.Session):
|
||||
session.env["PATH"] = venv_bin + os.pathsep + os.environ.get("PATH", "")
|
||||
|
||||
# Ensure output directory exists (CI starts with a clean checkout)
|
||||
# and clear stale pabot worker artifacts from previous interrupted runs.
|
||||
os.makedirs("build/reports/robot", exist_ok=True)
|
||||
shutil.rmtree("build/reports/robot/pabot_results", ignore_errors=True)
|
||||
|
||||
# Pass the venv Python path explicitly so Run Process calls use it
|
||||
# instead of relying on PATH (which may not propagate to subprocesses).
|
||||
|
||||
@@ -8,6 +8,7 @@ Library indentation_library.py
|
||||
Resource ${CURDIR}/common.resource
|
||||
|
||||
*** Variables ***
|
||||
${PYTHON} python
|
||||
${TEST_PROJECT_NAME} test-db-project
|
||||
${TEST_PLAN_NAME} test-plan
|
||||
${TEST_FILE} test_file.py
|
||||
@@ -759,12 +760,18 @@ Run Python Script
|
||||
${temp_file}= Evaluate (lambda t: (__import__('os').close(t[0]), t[1])[-1])(__import__('tempfile').mkstemp(suffix='.py', dir='/tmp'))
|
||||
Create File ${temp_file} ${full_code}
|
||||
${result}= Run Process ${PYTHON} ${temp_file} timeout=180s stderr=STDOUT env:PYTHONWARNINGS=ignore env:PYTHONDONTWRITEBYTECODE=1
|
||||
|
freemo
commented
CONFLICT: Master already has **CONFLICT**: Master already has `timeout=180s` on this line but retains the original error handling (no retry). The retry logic you added is valuable and should be kept, but this file has a merge conflict. After rebasing onto master, apply the retry logic on top of master's current state (which already has the 180s timeout).
freemo
commented
Merge conflict: Master already has **Merge conflict**: Master already has `timeout=180s` here but with simpler error handling (fail immediately, no retry). The PR's retry logic is valuable and should be preserved, but this section needs to be rebased onto master's current state. After rebase, re-apply the retry pattern on top of master's `Run Python Script` keyword.
freemo
commented
Merge conflict: Master restructured this section — it moved **Merge conflict**: Master restructured this section — it moved `Remove File` before the IF block and simplified error handling. The PR's retry logic is valuable but conflicts with master's current structure. After rebase, re-apply the retry pattern on top of master's current `Run Python Script` keyword body.
freemo
commented
CONFLICT: Master already has After rebase, re-apply the retry logic on top of master's current Automated by CleverAgents Bot **CONFLICT**: Master already has `timeout=180s` here but keeps the simpler error handling pattern (fail immediately, `Remove File` before the IF block). The PR's retry logic is valuable but conflicts with master's current structure.
After rebase, re-apply the retry logic on top of master's current `Run Python Script` keyword. Note that master has `Remove File` **before** the IF block — the PR moves it after (which is correct for the retry pattern, since you need the temp file for the retry). Keep the PR's placement but rebase onto master's current context.
---
**Automated by CleverAgents Bot**
Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo
commented
Conflict with master: Master already has During rebase:
Automated by CleverAgents Bot **Conflict with master**: Master already has `timeout=180s` and keeps `Remove File` before the IF block with simple fail-on-error. The PR's retry logic is valuable, but the diff context has diverged.
During rebase:
1. Keep `timeout=180s` (both branches agree)
2. Re-apply the retry logic on top of master's current keyword structure
3. Move `Remove File` after the IF/END block (correct for retry — temp file must exist for the retry attempt)
---
**Automated by CleverAgents Bot**
Supervisor: PR Review | Agent: ca-pr-self-reviewer
|
||||
Remove File ${temp_file}
|
||||
# Check if process failed and log stderr if present
|
||||
# Retry once for transient worker pressure (e.g. SIGTERM under heavy CI load)
|
||||
IF ${result.rc} != 0
|
||||
Log Process failed with rc=${result.rc} stdout: ${result.stdout} WARN
|
||||
Fail Python script execution failed (rc=${result.rc}): ${result.stdout}
|
||||
Log First Python script attempt failed with rc=${result.rc}; retrying once. Output: ${result.stdout} WARN
|
||||
${retry}= Run Process ${PYTHON} ${temp_file} timeout=180s stderr=STDOUT env:PYTHONWARNINGS=ignore env:PYTHONDONTWRITEBYTECODE=1
|
||||
IF ${retry.rc} == 0
|
||||
${result}= Set Variable ${retry}
|
||||
ELSE
|
||||
Remove File ${temp_file}
|
||||
Fail Python script execution failed after retry (rc=${retry.rc}): ${retry.stdout}
|
||||
END
|
||||
END
|
||||
Remove File ${temp_file}
|
||||
# Extract just the number from the output (last line)
|
||||
${lines}= Split String ${result.stdout} \n
|
||||
${filtered_lines}= Create List
|
||||
|
||||
@@ -82,7 +82,7 @@ def _run_error_script(python_path: str, script: str) -> dict[str, object]:
|
||||
[python_path, "-c", script],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=90,
|
||||
timeout=120,
|
||||
|
freemo
commented
CONFLICT: Master independently bumped this timeout from **CONFLICT**: Master independently bumped this timeout from `30` → `90`. Your PR changes it to `120`. After rebasing, decide on the final value. If `90` is sufficient (it was chosen by another stabilization PR), keep `90`. If you have evidence that `120` is needed, document why in the commit message.
freemo
commented
Merge conflict: Master independently bumped this from **Merge conflict**: Master independently bumped this from `30` → `90`. The PR changes it to `120`. After rebase, reconcile to a single value. If 120s is justified by CI timeout evidence, keep it; otherwise adopt master's 90.
freemo
commented
Merge conflict: Master independently bumped this timeout from **Merge conflict**: Master independently bumped this timeout from `30` → `90`. This PR sets it to `120`. After rebase, you'll need to pick one value. If `120` is justified by CI evidence (e.g., observed timeouts at 90s), keep it and add a comment explaining why. Otherwise, adopt master's `90`.
freemo
commented
CONFLICT: Master independently bumped this from Automated by CleverAgents Bot **CONFLICT**: Master independently bumped this from `30` → `90`. The PR changes it to `120`. After rebase, pick one value and justify it. If `120` is needed based on CI evidence (e.g., observed timeouts at 90s), keep it. Otherwise, adopt master's `90`.
---
**Automated by CleverAgents Bot**
Supervisor: PR Review | Agent: ca-pr-self-reviewer
freemo
commented
Conflict with master: Master independently bumped this timeout from If you have CI evidence that 90s is insufficient (e.g., timeout failures in CI logs), keep Automated by CleverAgents Bot **Conflict with master**: Master independently bumped this timeout from `30` → `90`. This PR changes it to `120`. After rebase, you'll need to pick one value.
If you have CI evidence that 90s is insufficient (e.g., timeout failures in CI logs), keep `120`. Otherwise, adopt master's `90` to stay consistent with the already-merged change. Either way, document the rationale in the commit body.
---
**Automated by CleverAgents Bot**
Supervisor: PR Review | Agent: ca-pr-self-reviewer
|
||||
)
|
||||
return {
|
||||
"rc": result.returncode,
|
||||
|
||||
@@ -10,13 +10,14 @@ Link Child And Verify Tree
|
||||
... import json
|
||||
... from datetime import datetime, UTC
|
||||
... from sqlalchemy import create_engine, event
|
||||
... from sqlalchemy.pool import StaticPool
|
||||
... from sqlalchemy.orm import sessionmaker
|
||||
... from sqlalchemy.pool import StaticPool
|
||||
... from cleveragents.infrastructure.database.models import Base
|
||||
|
freemo
commented
Master already has the Master already has the `shared_session = factory()` and `lambda: shared_session` pattern but does NOT have the `StaticPool` import or `sqlite://` connection string change. After rebase, you'll need to add only the `StaticPool` changes on top of master's existing shared_session pattern. The `from sqlalchemy.pool import StaticPool` import and `create_engine("sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool)` changes are still needed.
|
||||
... from cleveragents.infrastructure.database.repositories import ResourceTypeRepository, ResourceRepository
|
||||
... from cleveragents.domain.models.core.resource_type import ResourceTypeSpec, ResourceKind, SandboxStrategy
|
||||
... from cleveragents.domain.models.core.resource import Resource, PhysVirt, ResourceCapabilities
|
||||
... engine = create_engine("sqlite:///:memory:", poolclass=StaticPool, connect_args={"check_same_thread": False})
|
||||
... engine = create_engine("sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool)
|
||||
... @event.listens_for(engine, "connect")
|
||||
... def _fk(conn, _): conn.cursor().execute("PRAGMA foreign_keys=ON")
|
||||
... Base.metadata.create_all(engine)
|
||||
@@ -47,13 +48,14 @@ Cycle Detection Rejects A To B To A
|
||||
${script}= Catenate SEPARATOR=\n
|
||||
... from datetime import datetime, UTC
|
||||
... from sqlalchemy import create_engine, event
|
||||
... from sqlalchemy.pool import StaticPool
|
||||
... from sqlalchemy.orm import sessionmaker
|
||||
... from sqlalchemy.pool import StaticPool
|
||||
... from cleveragents.infrastructure.database.models import Base
|
||||
... from cleveragents.infrastructure.database.repositories import ResourceTypeRepository, ResourceRepository, CycleDetectedError
|
||||
... from cleveragents.domain.models.core.resource_type import ResourceTypeSpec, ResourceKind, SandboxStrategy
|
||||
... from cleveragents.domain.models.core.resource import Resource, PhysVirt, ResourceCapabilities
|
||||
... engine = create_engine("sqlite:///:memory:", poolclass=StaticPool, connect_args={"check_same_thread": False})
|
||||
... engine = create_engine("sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool)
|
||||
... @event.listens_for(engine, "connect")
|
||||
... def _fk(conn, _): conn.cursor().execute("PRAGMA foreign_keys=ON")
|
||||
... Base.metadata.create_all(engine)
|
||||
@@ -87,13 +89,14 @@ Auto Discover Children
|
||||
... import json
|
||||
... from datetime import datetime, UTC
|
||||
... from sqlalchemy import create_engine, event
|
||||
... from sqlalchemy.pool import StaticPool
|
||||
... from sqlalchemy.orm import sessionmaker
|
||||
... from sqlalchemy.pool import StaticPool
|
||||
... from cleveragents.infrastructure.database.models import Base
|
||||
... from cleveragents.infrastructure.database.repositories import ResourceTypeRepository, ResourceRepository
|
||||
... from cleveragents.domain.models.core.resource_type import ResourceTypeSpec, ResourceKind, SandboxStrategy
|
||||
... from cleveragents.domain.models.core.resource import Resource, PhysVirt, ResourceCapabilities
|
||||
... engine = create_engine("sqlite:///:memory:", poolclass=StaticPool, connect_args={"check_same_thread": False})
|
||||
... engine = create_engine("sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool)
|
||||
... @event.listens_for(engine, "connect")
|
||||
... def _fk(conn, _): conn.cursor().execute("PRAGMA foreign_keys=ON")
|
||||
... Base.metadata.create_all(engine)
|
||||
|
||||
@@ -180,12 +180,21 @@ Test Context File Not Created On Multiple Runs
|
||||
*** Keywords ***
|
||||
Setup Test Environment
|
||||
[Documentation] Setup test environment
|
||||
Create Directory ${TEST_DIR}
|
||||
Create Directory ${CONTEXT_DIR}
|
||||
|
||||
# Generate unique ID for this test run
|
||||
# Generate unique ID for this test run first, then scope all temp paths
|
||||
# to that ID so parallel suites cannot collide on shared temp files.
|
||||
${timestamp} = Get Time epoch
|
||||
Set Suite Variable ${UNIQUE_ID} ${timestamp}
|
||||
${test_dir} = Set Variable ${TEMPDIR}/rxpy_validation_test_${timestamp}
|
||||
${rxpy_config} = Set Variable ${test_dir}/rxpy_config.yaml
|
||||
${langgraph_config} = Set Variable ${test_dir}/langgraph_config.yaml
|
||||
${context_dir} = Set Variable ${test_dir}/test_contexts
|
||||
Set Suite Variable ${TEST_DIR} ${test_dir}
|
||||
Set Suite Variable ${RXPY_CONFIG} ${rxpy_config}
|
||||
Set Suite Variable ${LANGGRAPH_CONFIG} ${langgraph_config}
|
||||
Set Suite Variable ${CONTEXT_DIR} ${context_dir}
|
||||
|
||||
Create Directory ${TEST_DIR}
|
||||
Create Directory ${CONTEXT_DIR}
|
||||
|
||||
Cleanup Test Environment
|
||||
[Documentation] Clean up test environment
|
||||
|
||||
Redundant change: Master's
_default_processes()already returnsmin(cpus, 2), so wrapping the result inmin(2, _default_processes())here has no effect. Drop this change during rebase. Theshutilimport andpabot_resultscleanup below are still needed.Redundant change: Master's
_default_processes()already returnsmin(cpus, 2)(line 28). Wrapping this in anothermin(2, _default_processes())is a no-op — it evaluates tomin(2, min(cpus, 2))which always equalsmin(cpus, 2).Action: Drop this change during rebase. Keep only the
shutilimport andpabot_resultscleanup (those are still valuable).Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer