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
25 changes: 25 additions & 0 deletions dev-packages/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
.PHONY: run browser node node-core e2e

# Fuzzy-pick which test suite to run, then fuzzy-pick a test within it
run:
@if ! command -v fzf > /dev/null 2>&1; then \
echo "Error: fzf is required. Install with: brew install fzf"; \
exit 1; \
fi
@suite=$$(printf '%s\n' browser-integration-tests node-integration-tests node-core-integration-tests e2e-tests | \
fzf --height=10 --layout=reverse --border=rounded --margin=1.5% \
--color=dark --prompt="run test suite: "); \
[ -n "$$suite" ] && $(MAKE) -C $$suite run

# Run directly in a specific suite
browser:
@$(MAKE) -C browser-integration-tests run

node:
@$(MAKE) -C node-integration-tests run

node-core:
@$(MAKE) -C node-core-integration-tests run

e2e:
@$(MAKE) -C e2e-tests run
9 changes: 9 additions & 0 deletions dev-packages/browser-integration-tests/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.PHONY: run

run:
@if ! command -v fzf > /dev/null 2>&1; then \
echo "Error: fzf is required. Install with: brew install fzf"; \
exit 1; \
fi
@find . -name test.ts | sed -e 's|^\./suites/||' -e 's|/test\.ts$$||' | fzf --height=10 --layout=reverse --border=rounded --margin=1.5% --color=dark --prompt="yarn test " | xargs yarn test

4 changes: 2 additions & 2 deletions dev-packages/e2e-tests/Makefile
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
.PHONY: run list

run:
@if ! command -v fzf &> /dev/null; then \
@if ! command -v fzf > /dev/null 2>&1; then \
echo "Error: fzf is required. Install with: brew install fzf"; \
exit 1; \
fi
@ls test-applications | fzf --height=10 --layout=reverse --border=rounded --margin=1.5% --color=dark --prompt="yarn test:run " | xargs -r yarn test:run
@ls test-applications | fzf --height=10 --layout=reverse --border=rounded --margin=1.5% --color=dark --prompt="yarn test:run " | xargs yarn test:run
Copy link

Choose a reason for hiding this comment

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

Bug: Canceling the fzf test selection prompt unexpectedly runs all tests because the xargs command is missing the -r flag.
Severity: MEDIUM

Suggested Fix

Add the -r (or --no-run-if-empty) flag to the xargs command in the affected Makefile targets. This will ensure that if fzf provides no input (as happens on cancellation), the test command will not be executed. For example, ... | xargs yarn test:run should be changed to ... | xargs -r yarn test:run.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: dev-packages/e2e-tests/Makefile#L8

Potential issue: In several `Makefile` targets, the output of an `fzf` command is piped
to `xargs` to run a specific test. However, the `xargs` command is missing the `-r`
(`--no-run-if-empty`) flag. When a user cancels the `fzf` selection prompt (e.g., by
pressing ESC), `fzf` produces no output. Without the `-r` flag, `xargs` proceeds to
execute the test command (e.g., `yarn test:run`) with no arguments. This causes the test
runner's default behavior, which is to run all tests, contrary to the user's intent to
cancel the operation.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not run all the tests, fzf cancels out.

Copy link

Choose a reason for hiding this comment

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

Missing xargs guard runs all tests on cancel

Medium Severity

Removing -r from xargs in the e2e Makefile (and omitting it in the new Makefiles) means that on Linux (GNU xargs), if the user cancels fzf (presses Escape), xargs still executes yarn test / yarn test:run with no arguments — unintentionally running the entire test suite. BSD xargs on macOS skips execution on empty input by default, but GNU xargs does not. The parent Makefile correctly guards against empty selection with [ -n "$$suite" ], but these sub-Makefiles lack a similar guard.

Additional Locations (2)

Fix in Cursor Fix in Web


list:
@ls test-applications
9 changes: 9 additions & 0 deletions dev-packages/node-core-integration-tests/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.PHONY: run

run:
@if ! command -v fzf > /dev/null 2>&1; then \
echo "Error: fzf is required. Install with: brew install fzf"; \
exit 1; \
fi
@find . -name test.ts | sed -e 's|^\./suites/||' -e 's|/test\.ts$$||' | fzf --height=10 --layout=reverse --border=rounded --margin=1.5% --color=dark --prompt="yarn test " | xargs yarn test

9 changes: 9 additions & 0 deletions dev-packages/node-integration-tests/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.PHONY: run

run:
@if ! command -v fzf > /dev/null 2>&1; then \
echo "Error: fzf is required. Install with: brew install fzf"; \
exit 1; \
fi
@find . -name test.ts | sed -e 's|^\./suites/||' -e 's|/test\.ts$$||' | fzf --height=10 --layout=reverse --border=rounded --margin=1.5% --color=dark --prompt="yarn test " | xargs yarn test

Loading