Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .claude/commands/red.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,21 @@ This phase is **not part of the regular TDD workflow** and must only be applied
- Once sufficient understanding is achieved, all spike code is discarded, and normal TDD resumes starting from the **Red Phase**.
- A Spike is justified only when it is impossible to define a meaningful failing test due to technical uncertainty or unknown system behavior.

### If a New Test Passes Immediately

If a newly written test passes without any implementation change, do not assume it is correct. Verify it actually exercises the intended behavior:

1. Identify the implementation line most likely responsible for the pass
2. Temporarily remove that line
3. Run the **full test suite** (not just the new test)
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use targeted test execution first instead of mandating full-suite runs.

Line 103 conflicts with your established testing workflow: isolate the specific new test first, then widen scope. Forcing full-suite runs here will increase cycle time and can obscure the immediate red/green signal.

Suggested wording change
-3. Run the **full test suite** (not just the new test)
+3. Run the **new test in isolation first** to confirm it fails for the expected reason after removing the line.
+4. If isolation confirms behavior, then run the broader relevant suite (and full suite when needed) before finalizing.

Based on learnings: "When iterating on a single test, run that test in isolation first … Only run the full suite once the target test behaves as expected."

Also applies to: 107-109

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/red.md at line 103, Replace the imperative "Run the **full
test suite** (not just the new test)" with guidance to run the new/target test
in isolation first and only run the full suite after that test is passing;
specifically, change the phrase "Run the **full test suite**" to something like
"Run the new/target test in isolation first; run the full test suite only after
it passes" and make the same edit for the similar guidance around lines 107–109
(i.e., any occurrences of "Run the full test suite" or that bullet).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this whole set of steps only gets executed if the single test unexpectedly passes right? we already know the result of that one test

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ejfine, you're right — that's a fair point. The entire "If a New Test Passes Immediately" workflow is only triggered when the new test has already unexpectedly passed, so its result is already known. The purpose of removing the line and running the suite is specifically to detect whether other existing tests also depend on that line — which requires a broader run. Running only the new test in isolation after removing the line wouldn't surface that information. My suggestion was a misapplication of the "isolate first" guideline to a context where it doesn't fit. The existing wording is correct.


🧠 Learnings used
Learnt from: CR
Repo: LabAutomationAndScreening/copier-base-template PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-30T22:03:28.288Z
Learning: Applies to tests/**/*.py : When iterating on a single test, run that test in isolation first and confirm it is in the expected state (red or green) before widening to the full suite. Use the most targeted invocation available: a specific test function (e.g. `uv run pytest path/to/test.py::test_name --no-cov`). Only run the full suite once the target test behaves as expected.

Learnt from: CR
Repo: LabAutomationAndScreening/copier-base-template PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-30T22:03:28.288Z
Learning: Applies to tests/**/*.{ts,tsx} : When iterating on a single test, run that test in isolation first and confirm it is in the expected state (red or green) before widening to the full suite. Use the most targeted invocation available: a file path and name filter for TypeScript (e.g. `pnpm test-unit -- path/to/test.spec.ts -t "test name" --no-coverage`). Only run the full suite once the target test behaves as expected.

Learnt from: CR
Repo: LabAutomationAndScreening/copier-base-template PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-30T22:03:28.288Z
Learning: Applies to tests/**/*.{py,ts,tsx,js,jsx} : Test coverage requirements are usually at 100%, so when running a subset of tests, always disable test coverage to avoid the test run failing for insufficient coverage.

Learnt from: CR
Repo: LabAutomationAndScreening/copier-base-template PR: 0
File: template/template/CLAUDE.md:0-0
Timestamp: 2026-03-02T20:36:53.870Z
Learning: Applies to template/template/**/*.{test,spec}.{js,ts,jsx,tsx} : Write tests for critical functionality and maintain adequate test coverage

Learnt from: CR
Repo: LabAutomationAndScreening/copier-base-template PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-30T22:03:28.288Z
Learning: Applies to tests/**/*.{py,ts,tsx,js,jsx} : Avoid loops in tests — assert each item explicitly so failures pinpoint the exact element. When verifying a condition across all items in a collection, collect the violations into a list and assert it's empty (e.g., assert [x for x in items if bad_condition(x)] == []).


Then interpret the result:

- **Only the new test fails** — the line was never driven by a prior test. This is accidental over-implementation: delete the line permanently and proceed to the green phase to reintroduce it properly.
- **Other existing tests also fail** — the line was already legitimately required by prior work. The new test is valid regression coverage. Restore the line; the test is confirmed correct as written.

In both cases, confirm the new test fails for the expected reason before proceeding (the right assertion, not a syntax or import error).

### General Information

- Sometimes the test output shows as no tests have been run when a new test is failing due to a missing import or constructor. In such cases, allow the agent to create simple stubs. Ask them if they forgot to create a stub if they are stuck.
Expand Down
2 changes: 1 addition & 1 deletion .devcontainer/devcontainer.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,5 +75,5 @@
"initializeCommand": "sh .devcontainer/initialize-command.sh",
"onCreateCommand": "sh .devcontainer/on-create-command.sh",
"postStartCommand": "sh .devcontainer/post-start-command.sh"
// Devcontainer context hash (do not manually edit this, it's managed by a pre-commit hook): 5ecc43b1 # spellchecker:disable-line
// Devcontainer context hash (do not manually edit this, it's managed by a pre-commit hook): 4d399793 # spellchecker:disable-line
}
14 changes: 10 additions & 4 deletions .devcontainer/manual-setup-deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

REPO_ROOT_DIR = Path(__file__).parent.parent.resolve()
ENVS_CONFIG = REPO_ROOT_DIR / ".devcontainer" / "envs.json"
PULUMI_CLI_INSTALL_SCRIPT = REPO_ROOT_DIR / ".devcontainer" / "install-pulumi-cli.sh"
UV_PYTHON_ALREADY_CONFIGURED = "UV_PYTHON" in os.environ
parser = argparse.ArgumentParser(description="Manual setup for dependencies in the repo")
_ = parser.add_argument(
Expand Down Expand Up @@ -140,10 +141,15 @@ def main():
and env.lock_file.exists()
and '"pulumi"' in env.lock_file.read_text()
):
_ = subprocess.run(
["sh", str(REPO_ROOT_DIR / ".devcontainer" / "install-pulumi-cli.sh"), str(env.lock_file)],
check=True,
)
if not PULUMI_CLI_INSTALL_SCRIPT.exists():
print(
f"Pulumi CLI install script not found at {PULUMI_CLI_INSTALL_SCRIPT}, skipping Pulumi CLI installation"
)
else:
_ = subprocess.run(
["sh", str(PULUMI_CLI_INSTALL_SCRIPT), str(env.lock_file)],
check=True,
)
elif env.package_manager == PackageManager.PNPM:
pnpm_command = ["pnpm", "install", "--dir", str(env.path)]
if env_check_lock:
Expand Down
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ This project is a Copier template used to generate other copier templates. It is
- Avoid magic values in comparisons in tests in all languages (like ruff rule PLR2004 specifies)
- Prefer using random values in tests rather than arbitrary ones (e.g. the faker library, uuids, random.randint) when possible. For enums, pick randomly rather than hardcoding one value.
- Avoid loops in tests — assert each item explicitly so failures pinpoint the exact element. When verifying a condition across all items in a collection, collect the violations into a list and assert it's empty (e.g., assert [x for x in items if bad_condition(x)] == []).
- When a test's final assertion is an absence (e.g., element is `null`, list is empty, modal is closed), include a prior presence assertion confirming the expected state existed before the action that removed it. A test whose only assertion is an absence check can pass vacuously if setup silently failed.
- When asserting a mock or spy was called with specific arguments, always constrain as tightly as possible. In order of preference: (1) assert called exactly once with those args (`assert_called_once_with` in Python, `toHaveBeenCalledExactlyOnceWith` in Vitest/Jest); (2) if multiple calls are expected, assert the total call count and use a positional or last-call assertion (`nthCalledWith`, `lastCalledWith` / `assert_has_calls` with `call_args_list[n]`); (3) plain "called with at any point" (`toHaveBeenCalledWith`, `assert_called_with`) is a last resort only when neither the call count nor the call order can reasonably be constrained.

### Python Testing
Expand Down
1 change: 1 addition & 0 deletions template/.github/workflows/ci.yaml.jinja-base
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ jobs:

check-skip-duplicate:
runs-on: {% endraw %}{{ gha_linux_runner }}{% raw %}
timeout-minutes: {% endraw %}{{ gha_short_timeout_minutes }}{% raw %}
outputs:
should-run: ${{ steps.check.outputs.should-run }}
steps:
Expand Down
1 change: 1 addition & 0 deletions template/template/.github/workflows/ci.yaml.jinja
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ jobs:

check-skip-duplicate:
runs-on: {% endraw %}{{ gha_linux_runner }}{% raw %}
timeout-minutes: {% endraw %}{{ gha_short_timeout_minutes }}{% raw %}
permissions:
contents: read
pull-requests: read # needed to check if PR exists for current branch
Expand Down