Add regression coverage for automation review-thread fixes

[Copilot]

This PR captures the behaviors called out in review thread 4595682762 with focused regression tests, so the reviewed fixes remain enforced across the automation package. The coverage targets the generator variable, component registry, engine failure path, and package quick-start import example.

• Generator variable drain semantics

  • Adds coverage for the peek() + collect() interaction to ensure a peeked value is preserved when draining the remaining sequence.

• Component lifecycle and failure reporting

  • Verifies component replacement triggers teardown on the previously registered instance.
  • Verifies FunctionComponent surfaces traceback text when wrapped callables fail.

• Automation engine variable handling

  • Verifies missing declared variables fail the run instead of being silently omitted from the metadata snapshot.

• Package quick-start contract

  • Verifies the package docstring examples use automation imports rather than a non-package src.automation path.

variable = GeneratorVariable.from_iterable([1, 2, 3])

assert variable.peek() == 1
assert variable.collect() == [1, 2, 3]

[qodo-code-review]

PR Summary by Qodo

Add regression tests for automation review-thread fixes

🧪 Tests 🕐 10-20 Minutes

AI Description

• Add regression coverage for GeneratorVariable peek/collect drain semantics.
• Verify component replacement triggers teardown and FunctionComponent returns traceback on failure.
• Assert missing declared variables fail runs and quick-start docs use package imports.

Diagram

graph TD
  T["Review regression tests"] -->|"peek+collect preserves value"| GV["GeneratorVariable"] -->|"peek used during snapshot"| AE["AutomationEngine"]
  T -->|"replace triggers teardown"| CR["ComponentRegistry"]
  T -->|"traceback on failure"| FC["FunctionComponent"]
  T -->|"quick-start import contract"| DOC["automation __doc__"]

Loading

High-Level Assessment

Adding focused regression tests is the most appropriate way to preserve the reviewed behavioral contracts without expanding production surface area. Splitting into multiple test files was considered but is unnecessary given the small size and cohesive “review regressions” scope.

Files changed (1) +83 / -0

Tests (1) +83 / -0

test_automation_review_regressions.pyAdd regression tests for variables, registry lifecycle, engine failures, and docs +83/-0

Add regression tests for variables, registry lifecycle, engine failures, and docs

• Adds a dedicated regression test module covering: (1) GeneratorVariable.peek()+collect() preserving the peeked item, (2) ComponentRegistry replacement calling teardown on the previous component, (3) FunctionComponent returning traceback text on exceptions, (4) AutomationEngine failing runs when declared variables are missing, and (5) package docstring quick-start using 'automation' imports (not 'src.automation').

tests/test_automation_review_regressions.py

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. Docstring test brittleness 🐞 Bug ☼ Reliability

Description

test_quick_start_uses_package_imports hard-requires automation.__doc__ to be a non-empty str and to
contain specific text, so it can fail in environments where docstrings are stripped or if the
quick-start formatting changes without changing the intended import contract. This makes the
regression coverage less reliable than necessary.

Code

tests/test_automation_review_regressions.py[R79-83]

+def test_quick_start_uses_package_imports():
+    assert isinstance(automation_doc, str)
+    assert automation_doc.strip()
+    assert "from automation import AutomationEngine" in automation_doc
+    assert "from src.automation import" not in automation_doc

Evidence

The test currently imports the runtime docstring (automation.__doc__) and asserts it is a
non-empty string containing specific substrings. The quick-start text being asserted comes from the
module docstring in src/automation/__init__.py, so validating against the file/source content
would enforce the same contract without depending on __doc__ at runtime.

tests/test_automation_review_regressions.py[1-4]
tests/test_automation_review_regressions.py[79-83]
src/automation/init.py[1-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`test_quick_start_uses_package_imports` asserts against `automation.__doc__` directly. This couples the test to runtime docstring availability and exact formatting, making it easy to fail in certain execution modes or after harmless docstring edits.

### Issue Context
The quick-start content is defined in the module docstring of `src/automation/__init__.py`. The regression goal is to ensure the *example imports* reference `automation` (package import) rather than `src.automation`.

### Fix Focus Areas
- tests/test_automation_review_regressions.py[1-4]
- tests/test_automation_review_regressions.py[79-83]

### Suggested fix
Update the test to assert against the module *source text* instead of `__doc__`, e.g.:
- `import automation, pathlib`
- `text = pathlib.Path(automation.__file__).read_text(encoding="utf-8")`
- Assert the expected quick-start import lines are present/absent within `text`.

(Alternative: if you intentionally want to validate the runtime docstring, add a guard like `if automation_doc is None: pytest.skip(...)`.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[qodo-code-review]

1. Docstring test brittleness 🐞 Bug ☼ Reliability

test_quick_start_uses_package_imports hard-requires automation.__doc__ to be a non-empty str and to
contain specific text, so it can fail in environments where docstrings are stripped or if the
quick-start formatting changes without changing the intended import contract. This makes the
regression coverage less reliable than necessary.

Agent Prompt

### Issue description
`test_quick_start_uses_package_imports` asserts against `automation.__doc__` directly. This couples the test to runtime docstring availability and exact formatting, making it easy to fail in certain execution modes or after harmless docstring edits.

### Issue Context
The quick-start content is defined in the module docstring of `src/automation/__init__.py`. The regression goal is to ensure the *example imports* reference `automation` (package import) rather than `src.automation`.

### Fix Focus Areas
- tests/test_automation_review_regressions.py[1-4]
- tests/test_automation_review_regressions.py[79-83]

### Suggested fix
Update the test to assert against the module *source text* instead of `__doc__`, e.g.:
- `import automation, pathlib`
- `text = pathlib.Path(automation.__file__).read_text(encoding="utf-8")`
- Assert the expected quick-start import lines are present/absent within `text`.

(Alternative: if you intentionally want to validate the runtime docstring, add a guard like `if automation_doc is None: pytest.skip(...)`.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

[Pmaster-dev]

fsspec, black, tox?

[Copilot]

Pull request overview

Adds a focused regression test suite for the automation package to lock in behaviors discussed in review thread 4595682762, covering generator-variable drain semantics, component lifecycle behavior, engine failure behavior for missing variables, and the package quick-start import example.

Changes:

• Adds regression coverage for GeneratorVariable.peek() + collect() to ensure a peeked value is not lost when draining.
• Adds regression coverage for component registry replacement teardown and for FunctionComponent returning traceback text on callable failure.
• Adds regression coverage that missing declared variables fail an automation run, and that the automation package docstring quick-start uses from automation import … imports.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.