feat: TestingBase pilot — submodule + XML validation + plugin smoke (2026.5.4)#58
Open
simons-plugins wants to merge 14 commits into
Open
feat: TestingBase pilot — submodule + XML validation + plugin smoke (2026.5.4)#58simons-plugins wants to merge 14 commits into
simons-plugins wants to merge 14 commits into
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThis PR establishes a two-environment integration testing framework by adding a TestingBase submodule for live HTTP tests against an Indigo server, with XML validation and plugin-load verification, separate pytest configuration to isolate integration tests from main unit tests, and documentation of the new testing pattern. ChangesIntegration Testing Infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Merged
3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tests/shared (1)
1-1: 💤 Low valueSubmodule pinned to a specific commit — LGTM.
Pinning
tests/sharedto an explicit commit SHA (3fe47ef) rather than a moving ref is the right call for reproducible integration runs. As an optional follow-up, if the upstreamTestingBaserepo publishes tagged releases, consider tracking a tag instead of a commit onmainso submodule bumps map to versioned upstream changes; otherwise this is fine as-is.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/shared` at line 1, No code change required—the submodule "tests/shared" is correctly pinned to commit 3fe47ef for reproducible runs; if you want an optional follow-up, switch the submodule ref from the commit SHA to an upstream release tag (if TestingBase publishes tags) by updating the gitlink to that tag and committing the submodule change so future bumps map to versioned releases instead of a raw commit..gitignore (1)
53-55: 💤 Low valueRedundant entries — already covered by the top-level patterns.
tests/.envis already ignored by.env(Line 42), andtests/venv/is already ignored byvenv/(Line 8) — git applies both patterns recursively to all subdirectories. The new entries are harmless but add noise.♻️ Simplification (keeping the intent clear via comment alone)
-# TestingBase — gitignored test creds and dedicated venv -tests/.env -tests/venv/ +# TestingBase — tests/.env and tests/venv/ are already excluded by .env and venv/ above🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitignore around lines 53 - 55, Remove the redundant specific ignore lines for tests/.env and tests/venv/ from .gitignore because they are already covered by the global patterns .env and venv/ (which apply recursively); update the file by deleting the two lines "tests/.env" and "tests/venv/" and, if needed, keep or add a brief comment indicating that .env and venv/ cover test subdirectories to preserve intent.tests/integration/test_indigo_smoke.py (1)
26-36: 💤 Low value
f'state=found'has an f-prefix with no interpolation — minor anti-pattern.The generated line
f'state=found'(line 31 of the script source) is a redundant f-string. It's harmless and functionally correct since it's concatenated with the adjacent f-strings that do have interpolations, but a plain'state=found'is cleaner.♻️ Remove spurious f-prefix
- f" f'state=found'\n" + f" 'state=found'\n"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/test_indigo_smoke.py` around lines 26 - 36, The script string uses a redundant f-string for the literal "state=found"; in the multi-line construction assigned to variable script (where plugin is fetched via indigo.server.getPlugin(self.plugin_id) and plugin.isInstalled()/isEnabled()/isRunning() are interpolated), change f'state=found' to a plain 'state=found' to remove the spurious f-prefix while leaving the other f-strings intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/test_xml_validation.py`:
- Around line 1-6: Update the module docstring in
tests/integration/test_xml_validation.py to clarify that although the Indigo
server process need not be manually started, APIBase.setUpClass runs in the test
MRO and spawns the indigo-host process, so Indigo must be installed locally for
the tests to run; reference APIBase.setUpClass and indigo-host in the text so
contributors understand the actual installation requirement.
---
Nitpick comments:
In @.gitignore:
- Around line 53-55: Remove the redundant specific ignore lines for tests/.env
and tests/venv/ from .gitignore because they are already covered by the global
patterns .env and venv/ (which apply recursively); update the file by deleting
the two lines "tests/.env" and "tests/venv/" and, if needed, keep or add a brief
comment indicating that .env and venv/ cover test subdirectories to preserve
intent.
In `@tests/integration/test_indigo_smoke.py`:
- Around line 26-36: The script string uses a redundant f-string for the literal
"state=found"; in the multi-line construction assigned to variable script (where
plugin is fetched via indigo.server.getPlugin(self.plugin_id) and
plugin.isInstalled()/isEnabled()/isRunning() are interpolated), change
f'state=found' to a plain 'state=found' to remove the spurious f-prefix while
leaving the other f-strings intact.
In `@tests/shared`:
- Line 1: No code change required—the submodule "tests/shared" is correctly
pinned to commit 3fe47ef for reproducible runs; if you want an optional
follow-up, switch the submodule ref from the commit SHA to an upstream release
tag (if TestingBase publishes tags) by updating the gitlink to that tag and
committing the submodule change so future bumps map to versioned releases
instead of a raw commit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19a14d3e-17b9-4575-a61f-07ea67b3e2b6
📒 Files selected for processing (15)
.gitignore.gitmodulesNetro Sprinklers.indigoPlugin/Contents/Info.plistdocs/plans/2026-05-04-netro-testingbase-pilot-design.mddocs/plans/2026-05-04-netro-testingbase-pilot-plan.mdpyproject.tomlpytest.initests/.env.exampletests/README.mdtests/integration/__init__.pytests/integration/pytest.initests/integration/test_indigo_smoke.pytests/integration/test_xml_validation.pytests/sharedtests/testing-requirements.txt
Comment on lines
+1
to
+6
| """ValidateXmlFile coverage for all 5 netro plugin XML files. | ||
|
|
||
| These tests don't need Indigo running — they validate the XML | ||
| files on disk against TestingBase's bundled schema. Catch broken | ||
| XML before it reaches end users. | ||
| """ |
There was a problem hiding this comment.
Docstring understates the Indigo installation requirement
"These tests don't need Indigo running" is technically true (the Indigo server needn't be up), but because APIBase is in the MRO, APIBase.setUpClass still executes and spawns indigo-host, which requires Indigo to be installed locally. The README already captures this accurately; the module docstring should match to avoid misleading a contributor who might attempt to run these tests in an environment without Indigo installed (e.g., CI).
📝 Proposed docstring clarification
-These tests don't need Indigo running — they validate the XML
-files on disk against TestingBase's bundled schema. Catch broken
-XML before it reaches end users.
+These tests validate XML files on disk against TestingBase's bundled
+schema; the Indigo *server* does not need to be running. However,
+``APIBase`` is in the MRO so ``APIBase.setUpClass`` still executes
+and requires Indigo to be installed locally (it spawns ``indigo-host``
+to resolve the install path). Run from the dedicated integration venv.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| """ValidateXmlFile coverage for all 5 netro plugin XML files. | |
| These tests don't need Indigo running — they validate the XML | |
| files on disk against TestingBase's bundled schema. Catch broken | |
| XML before it reaches end users. | |
| """ | |
| """ValidateXmlFile coverage for all 5 netro plugin XML files. | |
| These tests validate XML files on disk against TestingBase's bundled | |
| schema; the Indigo *server* does not need to be running. However, | |
| ``APIBase`` is in the MRO so ``APIBase.setUpClass`` still executes | |
| and requires Indigo to be installed locally (it spawns ``indigo-host`` | |
| to resolve the install path). Run from the dedicated integration venv. | |
| """ |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/integration/test_xml_validation.py` around lines 1 - 6, Update the
module docstring in tests/integration/test_xml_validation.py to clarify that
although the Indigo server process need not be manually started,
APIBase.setUpClass runs in the test MRO and spawns the indigo-host process, so
Indigo must be installed locally for the tests to run; reference
APIBase.setUpClass and indigo-host in the text so contributors understand the
actual installation requirement.
Captures the approved design before implementation: - Approach: upstream-strict two-venv layout (tests/shared submodule + tests/venv + tests/testing-requirements.txt). Picked over a pytest- unified single-venv approach because it works uniformly across all workspace plugins (most don't have pyproject.toml) and matches what other Indigo plugin developers use. - Pilot scope: ValidateXmlFile across all 5 netro XML files (Actions/Devices/Events/MenuItems/PluginConfig) plus one APIBase smoke test that fetches Indigo's plugin list and asserts netro is loaded. No netro hardware required. - Environment: local Indigo 2025.2 sandbox via dev license. jarvis stays on 2025.1 for now. - Intent: full workspace adoption. Pilot exists to learn the mechanics; rollout to heatmiser/home-intelligence/UK-Trains plus workspace CLAUDE.md update is follow-on work. Implementation plan to follow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
12 bite-sized tasks covering submodule setup, venv creation, env template, ValidateXmlFile coverage of all 5 netro XML files, the APIBase plugin-loaded smoke test, pytest config to keep main env clean, tests/README.md, and version bump + PR. Companion to the design doc in the same folder. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…test discovery The TestingBase pilot adds three subtrees under tests/ that the main pytest env can't import: - tests/integration/ — APIBase / ValidateXmlFile tests; require the dedicated venv - tests/shared/ — TestingBase submodule, ships its own example tests (example_test_xml_files.py) that fail without the dedicated venv - tests/venv/ — pytest-internal test files inside the venv itself Adding norecursedirs to both pytest.ini (active config) and pyproject.toml's [tool.pytest.ini_options] block (kept in sync for tooling that reads pyproject) so a casual `pytest` from netro's main env still passes cleanly. Verified: `pytest --collect-only` collects all 508 existing Pattern A tests across 13 files and 0 TestingBase tests. Note for follow-up: the duplication of pytest config across pytest.ini and pyproject.toml is pre-existing technical debt; consolidating to one file is a separate cleanup task. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related fixes the plan didn't anticipate: - tests/testing-requirements.txt: add `pytest`. Upstream's module-requirements.txt installs python-dotenv and httpx but not pytest, so the integration venv couldn't run the tests without a separate manual pip install. - tests/integration/pytest.ini: empty `addopts` so the integration suite ignores the main netro pytest.ini's coverage flags. Those flags require pytest-cov (which isn't in this venv) and produce meaningless output for integration tests anyway since they don't import netro source. Pytest's rootdir search picks up this inner config first when running tests under tests/integration/. Verified: from tests/, `venv/bin/pytest integration/ --collect-only` collects exactly the 6 expected tests (5 ValidateXmlFile + 1 APIBase smoke) with no command-line overrides needed. Note: this adds a third pytest config file in the repo. Unlike the pytest.ini/pyproject.toml duplication (pre-existing tech debt, same scope), this one is integration-scoped — different responsibility, not duplicate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…TP endpoint The original smoke test guessed at an `indigo.plugins` HTTP endpoint that doesn't exist. Indigo's HTTP API only exposes devices, variables, actionGroups, controlPages, logs, triggers, and schedules. Plugin state isn't queryable via HTTP at all. Switch to the IOM via `run_host_script` (TestingBase's wrapper around `indigo-host -e <script>`). Two corrections vs. the prior attempt: 1. Use `indigo.server.getPlugin(pluginId)` — documented in docs/plugin-dev/api/iom/command-namespaces.md (loaded via /indigo:dev). The returned object exposes isInstalled(), isEnabled(), isRunning(). 2. Use `return`, not `print`. `indigo-host -e` wraps the script as a function body and `print()` goes to Indigo's event log, not the subprocess stdout. Upstream's own tests/shared/utils.py:get_install_folder uses the same `return` convention. Result: 6/6 integration tests green against local Indigo 2025.2 via the clarkcastle-dev reflector (5 ValidateXmlFile + this smoke test), in about 15s. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
simons-plugins
force-pushed
the
feat/testingbase-pilot
branch
from
58f0335 to
3ae3b97
Compare
Owner
Author
|
Rebased onto main (workflow PR #59 merged). Bumped PluginVersion 2026.5.4 → 2026.5.6 because v2026.5.5 was just tagged by the workflow PR's release. Note: the existing 'chore: bump PluginVersion to 2026.5.4' commit message is now stale — actual bump is to 2026.5.6. PR title/description still reference 2026.5.4 but can be updated when convenient. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First adoption of TestingBase in this workspace. Establishes the upstream-strict two-venv layout as the workspace standard for live integration tests, validated end-to-end on local Indigo 2025.2.
Design and rationale:
docs/plans/2026-05-04-netro-testingbase-pilot-design.md(committed in this branch).What's added
TestingBase scaffolding:
tests/shared/— TestingBase submodule pinned to upstream main (3fe47ef)tests/testing-requirements.txt— chains toshared/module-requirements.txtplus addspytest(upstream's chain doesn't include it)tests/.env.example— committed template; real.envis gitignoredtests/integration/__init__.py— package marker with run instructionsLive tests (run via dedicated venv):
tests/integration/test_xml_validation.py— 5ValidateXmlFileclasses covering Actions/Devices/Events/MenuItems/PluginConfig.xml. Schema validation, no Indigo runtime needed for the validation itself.tests/integration/test_indigo_smoke.py— singleAPIBasetest assertingcom.simons-plugins.netrois loaded and enabled in the running Indigo server.tests/integration/pytest.ini— emptyaddoptsso the integration suite isolates from the main env's coverage flags (which requirepytest-cov, deliberately not in the integration venv).Wiring:
pytest.iniandpyproject.toml:norecursedirs = tests/integration tests/shared tests/venv. Verified —pytest --collect-onlyfrom main env collects all 508 Pattern A tests across 13 files and 0 TestingBase tests..gitignore: addstests/.envandtests/venv/.tests/README.md: documents Pattern A + Pattern B for new contributors.Delivery:
PluginVersion2026.5.3 → 2026.5.4 (patch — test infra change, no user-facing behaviour).Test plan
pytestfrom main env (Pattern A): all 508 existing tests still discovered and pass; 0 TestingBase tests collectedsource tests/venv/bin/activate && pytest tests/integration/: 6 tests pass (5 XML + 1 smoke). Author runs locally — sandbox can't reach/usr/local/indigo/indigo-hostenabledfield doesn't discriminate and we need a different key (per code review feedback)git submodule statusshowstests/sharedinitialised at3fe47eftests/.envis gitignored,tests/.env.exampleis committedOut of scope (deferred)
/usr/local/indigo/indigo-host) which CI runners don't have. CI continues to run only Pattern A.CLAUDE.mdstandard update — separate PR after this lands.Follow-up notes (out of scope here)
pytest.iniandpyproject.toml [tool.pytest.ini_options]. Pre-existing tech debt; consolidating is its own cleanup task. We addednorecursedirsto both to keep them in sync.tests/README.mdhardcodes the python.org Python 3.13 framework path. Developers using Homebrew Python 3.13 will need to substitute. Could be polished to a less prescriptive instruction in a follow-up.enabledas the discriminating field on Indigo's plugin object. Need to verify on first green run that disabling the plugin actually flips this — if not, switch to whichever field is right.Branch structure
12 commits on top of main, in logical order: design → plan → infra setup → tests → wiring → docs → version bump. Each commit is small and reviewable on its own.
Summary by CodeRabbit
Release Notes