Skip to content

Fixed workflow tests. #2073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AlexSkrypnyk merged 1 commit into from
Nov 1, 2025
Merged

Fixed workflow tests. #2073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 18 additions & 1 deletion .github/workflows/vortex-test-common.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,15 @@ jobs:
- name: Adjust git config to allow running git-related tests
run: git config --global safe.directory '*'

- name: Generate test SSH key pair used for deployment tests
run: |
mkdir -p "$HOME/.ssh"
ssh-keygen -t rsa -b 4096 -m PEM -N '' -f "$HOME/.ssh/id_rsa"
chmod 600 "$HOME/.ssh/id_rsa"
ssh-agent -a "${HOME}/ssh-agent.sock" > ssh-agent-output
export SSH_AUTH_SOCK=$(grep SSH_AUTH_SOCK ssh-agent-output | cut -d';' -f1 | cut -d= -f2)
echo "SSH_AUTH_SOCK=${SSH_AUTH_SOCK}" >> "$GITHUB_ENV"
Comment on lines +144 to +151

@coderabbitai coderabbitai Bot Nov 1, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Consider extracting SSH key generation to a composite action.

The SSH key generation logic at lines 144-151 is identical to lines 53-60 in the vortex-test-common job. While this duplication works, extracting it to a reusable composite action would improve maintainability and reduce the risk of the two implementations diverging.

🤖 Prompt for AI Agents 
.github/workflows/vortex-test-common.yml lines 144-151: the SSH key generation
block is duplicated elsewhere (lines 53-60); extract this logic into a reusable
composite action (e.g., .github/actions/generate-ssh-key/action.yml) that runs
the mkdir/ssh-keygen/chmod/ssh-agent steps and exposes SSH_AUTH_SOCK (and any
key paths) as outputs, then replace the two inlined blocks with a single step
using: uses: ./.github/actions/generate-ssh-key and map the output into the job
environment (echo to GITHUB_ENV or set as env), ensure the action sets proper
file permissions and returns any necessary outputs so both jobs consume the same
implementation.


- name: Setup Node.js
uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 # v6
with:
Expand All @@ -161,7 +170,15 @@ jobs:
run: |
# Create an empty coverage directory to avoid errors. @see https://github.com/actions/upload-artifact/issues/255
mkdir -p /tmp/.vortex-coverage-html && touch "/tmp/.vortex-coverage-html/.empty-$(date +%Y%m%d%H%M%S)"
php -d memory_limit=-1 vendor/bin/phpunit phpunit --group="${TEST_NODE_INDEX:-*}"

# Configure git username and email if it is not set.
[ "$(git config --global user.name)" = "" ] && echo "==> Configuring global test git user name" && git config --global user.name "Test user"
[ "$(git config --global user.email)" = "" ] && echo "==> Configuring global test git user email" && git config --global user.email "someone@example.com"
Comment on lines +174 to +176

@coderabbitai coderabbitai Bot Nov 1, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Use -z flag for clearer zero-length string checks.

The git config guard logic is correct but can be made more idiomatic by using the -z flag instead of = "" comparisons, which is the standard POSIX shell pattern for checking empty strings.

- [ "$(git config --global user.name)" = "" ] && echo "==> Configuring global test git user name" && git config --global user.name "Test user"
- [ "$(git config --global user.email)" = "" ] && echo "==> Configuring global test git user email" && git config --global user.email "someone@example.com"
+ [ -z "$(git config --global user.name)" ] && echo "==> Configuring global test git user name" && git config --global user.name "Test user"
+ [ -z "$(git config --global user.email)" ] && echo "==> Configuring global test git user email" && git config --global user.email "someone@example.com"
📝 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
# Configure git username and email if it is not set.
[ "$(git config --global user.name)" = "" ] && echo "==> Configuring global test git user name" && git config --global user.name "Test user"
[ "$(git config --global user.email)" = "" ] && echo "==> Configuring global test git user email" && git config --global user.email "someone@example.com"
# Configure git username and email if it is not set.
[ -z "$(git config --global user.name)" ] && echo "==> Configuring global test git user name" && git config --global user.name "Test user"
[ -z "$(git config --global user.email)" ] && echo "==> Configuring global test git user email" && git config --global user.email "someone@example.com"
🤖 Prompt for AI Agents 
.github/workflows/vortex-test-common.yml around lines 174 to 176: the current
checks use string equality to test for empty git config values; replace those
comparisons with the POSIX idiom using the -z test so the conditions read check
if the command substitution returns a zero-length string (e.g., [ -z "$(git
config --global user.name)" ] && ...), keeping the same echo and git config
assignment logic.


# Create stub of local network.
docker network create amazeeio-network 2>/dev/null || true

php -d memory_limit=-1 vendor/bin/phpunit --group="p${TEST_NODE_INDEX:-*}"
Comment on lines +179 to +181

@coderabbitai coderabbitai Bot Nov 1, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Docker network creation and phpunit grouping look sound.

The docker network stub creation with proper error suppression follows best practices, and the phpunit test grouping pattern p${TEST_NODE_INDEX:-*} correctly distributes tests across matrix batches (0–4). The fallback to * (all tests) for unset indices provides sensible defaults.

Verify that tests tagged with groups p0 through p4 exist and are properly distributed to ensure no test gaps or overlaps across batches:

🏁 Script executed:

#!/bin/bash
# Description: Verify phpunit test groups are properly defined and distributed

# Search for test group annotations in the test files
rg -n '@group\s+p[0-4]' .vortex/tests --type=php | head -20

# Count tests per group
echo "=== Test count per group ==="
rg '@group\s+p[0-4]' .vortex/tests --type=php -c | sort | uniq -c

Length of output: 28

I need to search more broadly to understand the actual test grouping strategy used in this repository.

🏁 Script executed:

#!/bin/bash
# Description: Search for all @group annotations to understand actual test grouping

echo "=== All @group annotations in .vortex/tests ==="
rg -n '@group' .vortex/tests --type=php -A1 | head -30

echo ""
echo "=== Unique group names ==="
rg '@group\s+(\S+)' .vortex/tests --type=php -o --replace '$1' | sort | uniq -c | sort -rn

echo ""
echo "=== Check for phpunit.xml or test configuration ==="
fd '(phpunit|test)" -type f .vortex/tests | head -20

Length of output: 198

Let me fix the script and gather more comprehensive information:

🏁 Script executed:

#!/bin/bash
# Description: Understand test structure and grouping

echo "=== Test files in .vortex/tests ==="
fd --type f '.php$' .vortex/tests | head -20

echo ""
echo "=== Search for @group in test files ==="
rg -n '@group' .vortex/tests --type=php | head -20

echo ""
echo "=== Check for phpunit.xml ==="
fd -name 'phpunit.xml*' .vortex

echo ""
echo "=== Check workflow context - lines around 179 ==="
sed -n '170,190p' .github/workflows/vortex-test-common.yml

Length of output: 2422

🏁 Script executed:

#!/bin/bash
# Description: Check phpunit config and TEST_NODE_INDEX usage

echo "=== Find phpunit configuration files ==="
find .vortex -name 'phpunit.xml*' -o -name 'phpunit.dist.xml' 2>/dev/null

echo ""
echo "=== Search for TEST_NODE_INDEX in workflow ==="
grep -n 'TEST_NODE_INDEX' .github/workflows/vortex-test-common.yml

echo ""
echo "=== Check matrix batch configuration ==="
sed -n '1,200p' .github/workflows/vortex-test-common.yml | grep -A 20 'strategy:'

echo ""
echo "=== Verify no @group tags exist at all in tests ==="
rg -c '^[[:space:]]*\*[[:space:]]*@group' .vortex/tests --type=php

Length of output: 1661

🏁 Script executed:

#!/bin/bash
# Description: Check phpunit.xml configuration

echo "=== phpunit.xml content ==="
cat .vortex/tests/phpunit.xml

echo ""
echo "=== Check if there are any group-related configurations ==="
grep -i 'group' .vortex/tests/phpunit.xml || echo "(no group configuration found)"

Length of output: 1327

Matrix test distribution is non-functional—tests lack required group annotations.

The workflow references --group="p${TEST_NODE_INDEX:-*}" with matrix batches [0–4], but no tests in .vortex/tests are tagged with @group p0 through @group p4. The phpunit.xml contains no group configuration. Consequently, all batches execute the complete test suite instead of distributing work across parallel jobs, defeating the matrix strategy's purpose.

Tests must be annotated with @group p0, @group p1, etc., to enable proper parallelization. Without these annotations, the workflow will continue running redundant full test suites in each batch.

🤖 Prompt for AI Agents 
.github/workflows/vortex-test-common.yml lines 179-181: the CI uses phpunit
--group="p${TEST_NODE_INDEX:-*}" to split tests across matrix nodes but no tests
are annotated with @group p0..p4 (and phpunit.xml has no group config), so each
matrix job runs the full suite; fix by annotating tests in .vortex/tests with
the appropriate group tags matching your matrix indices (e.g., add @group p0,
@group p1, … to test classes or methods) or alternatively update phpunit.xml to
define group aliases or implement a test-splitting script that assigns files to
groups consistently with the matrix; ensure the group names exactly match p0..p4
and run a local phpunit --group check to verify distribution.

working-directory: .vortex/tests
env:
TEST_VORTEX_DEBUG: ${{ vars.TEST_VORTEX_DEBUG }}
Expand Down
1 change: 1 addition & 0 deletions .vortex/tests/phpunit/Functional/InstallerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ protected function setUp(): void {
$this->gitAssertClean(static::$repo, 'Git working tree of the Vortex template repository should be clean');
}

#[Group('p0')]
public function testInstallFromLatest(): void {
$this->logSubstep('Add custom files to SUT');
File::dump('test1.txt', 'test content');
Expand Down