Skip to content

Conversation

@AlexSkrypnyk
Copy link
Member

@AlexSkrypnyk AlexSkrypnyk commented Sep 15, 2025

Related to #1560

Summary by CodeRabbit

  • New Tests

    • Added functional tests for Ahoy commands, Docker Compose workflow (including private package token handling), doctor info output, and idempotence.
    • Introduced a helper to download a demo database within test flows.
    • Renamed and reorganized install/database tests; removed obsolete workflow tests.
  • Documentation

    • Corrected README example to run the idempotence test.
  • Style

    • Updated coding standards to allow missing class comments in test files.
  • Refactor

    • Adjusted test setup logging and assertions for clarity during provisioning and volume-mount scenarios.

@coderabbitai
Copy link

coderabbitai bot commented Sep 15, 2025

Walkthrough

Reorganizes functional tests by deleting WorkflowTest and WorkflowUtilitiesTest, adding focused tests (Ahoy, DockerCompose, Doctor, Idempotence), and renaming InstallDbTest. Introduces a new DB download substep in FunctionalTestCase and removes the old trait method. Updates PHPCS rules for test comments and adjusts StepPrepareSutTrait logging/assertions. Updates README test example.

Changes

Cohort / File(s) Summary
Documentation
.vortex/tests/README.md
Example PHPUnit command now targets IdempotenceTest.php instead of WorkflowTest.php.
PHPCS configuration
.vortex/tests/phpcs.xml
Adds Drupal.Commenting.ClassComment.Missing with exclusions for test files; no public API changes.
Functional tests added
.vortex/tests/phpunit/Functional/AhoyTest.php, .vortex/tests/phpunit/Functional/DockerComposeTest.php, .vortex/tests/phpunit/Functional/DoctorTest.php, .vortex/tests/phpunit/Functional/IdempotenceTest.php
New tests for local Ahoy behavior, Docker Compose workflow and package token, doctor info output, and idempotence cycle.
Functional tests removed
.vortex/tests/phpunit/Functional/WorkflowTest.php, .vortex/tests/phpunit/Functional/WorkflowUtilitiesTest.php
Removes legacy workflow and utilities tests (smoke, idempotence, package token, Docker Compose without Ahoy, local Ahoy commands, doctor info).
Existing test modified
.vortex/tests/phpunit/Functional/InstallDbTest.php
Renames WorkflowInstallDbTest to InstallDbTest; switches setup from stepDownloadDb() to substepDownloadDb().
Test base/traits updates
.vortex/tests/phpunit/Functional/FunctionalTestCase.php, .vortex/tests/phpunit/Traits/Steps/StepDatabaseTrait.php
Adds protected substepDownloadDb(...) helper to FunctionalTestCase; removes stepDownloadDb() from StepDatabaseTrait.
SUT preparation adjustments
.vortex/tests/phpunit/Traits/Steps/StepPrepareSutTrait.php
Adds assertions to ensure special comments are absent post-install; changes some logSubstep calls to logNote; updates notes around unmounted-volumes patching.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant T as IdempotenceTest
  participant F as FunctionalTestCase
  participant B as Build/Provision
  participant G as Git
  participant BDD as BDD Tests

  T->>F: substepDownloadDb()
  T->>F: stepBuild()
  F->>B: Build project
  T->>F: assertFilesTrackedInGit()
  F->>G: Verify tracked files
  T->>F: stepAhoyTestBddFast()
  F->>BDD: Run fast BDD suite
  T->>F: stepBuild() (re-build)
  F->>B: Build project
  T->>F: assertFilesTrackedInGit(skip_commit=TRUE)
  F->>G: Verify tracked files
  T->>F: stepAhoyTestBddFast()
  F->>BDD: Run fast BDD suite
  note over T,F: Validates idempotence across rebuilds
Loading
sequenceDiagram
  autonumber
  participant T as DockerComposeTest
  participant DC as Docker Compose
  participant C as Containers (cli/web/...)
  participant P as Provision Script
  participant L as Linters/Tests

  T->>DC: docker compose build/up
  DC->>C: Start services
  T->>C: composer/yarn install
  T->>L: Run backend & frontend linters
  T->>P: Provision environment
  T->>L: Run PHPUnit/Behat
  alt Package token missing
    T->>DC: Build without PACKAGE_TOKEN
    DC-->>T: Fail
  else Package token provided
    T->>DC: Build with PACKAGE_TOKEN
    DC-->>T: Succeed
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

A rabbit taps keys with a grin so wide,
Old tests hop off, new ones by its side.
Idempotence twirls, Docker drums a beat,
Ahoy shouts “ready!”—what a tidy feat.
Tokens tucked snug, logs softly glow—
Onward we bound, to green pastures we go! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Split workflow tests." is concise, single-sentence, and accurately summarizes the primary change set, which reorganizes a monolithic workflow test into multiple focused functional test files and related adjustments; it clearly communicates the main intent for a teammate scanning history.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/bats2phpunit-4

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot temporarily deployed to commit September 15, 2025 03:49 Inactive
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
.vortex/tests/README.md (1)

73-79: Fix broken Markdown link formatting

The first link has mismatched backticks/brackets; it won’t render.

Apply this diff:

- - [`test.common.sh]`(test.common.sh) - Common tests for all environments
+ - [`test.common.sh`](test.common.sh) - Common tests for all environments
.vortex/tests/phpunit/Traits/Steps/StepPrepareSutTrait.php (1)

116-126: Use non‑TTY exec for CI robustness

Add “-T” to docker compose exec to avoid TTY issues in non‑interactive CI (consistent with FunctionalTestCase).

Apply this diff:

-      File::replaceContentInFile('.ahoy.yml',
-        '      ahoy cli ./scripts/vortex/provision.sh',
-        '      if [ -f .data/db.sql ]; then docker compose exec cli mkdir -p .data; docker compose cp -L .data/db.sql cli:/app/.data/db.sql; fi; ahoy cli ./scripts/vortex/provision.sh',
-      );
+      File::replaceContentInFile('.ahoy.yml',
+        '      ahoy cli ./scripts/vortex/provision.sh',
+        '      if [ -f .data/db.sql ]; then docker compose exec -T cli mkdir -p .data; docker compose cp -L .data/db.sql cli:/app/.data/db.sql; fi; ahoy cli ./scripts/vortex/provision.sh',
+      );
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 367866e and c50923f.

📒 Files selected for processing (12)
  • .vortex/tests/README.md (1 hunks)
  • .vortex/tests/phpcs.xml (1 hunks)
  • .vortex/tests/phpunit/Functional/AhoyTest.php (1 hunks)
  • .vortex/tests/phpunit/Functional/DockerComposeTest.php (1 hunks)
  • .vortex/tests/phpunit/Functional/DoctorTest.php (1 hunks)
  • .vortex/tests/phpunit/Functional/FunctionalTestCase.php (1 hunks)
  • .vortex/tests/phpunit/Functional/IdempotenceTest.php (1 hunks)
  • .vortex/tests/phpunit/Functional/InstallDbTest.php (2 hunks)
  • .vortex/tests/phpunit/Functional/WorkflowTest.php (0 hunks)
  • .vortex/tests/phpunit/Functional/WorkflowUtilitiesTest.php (0 hunks)
  • .vortex/tests/phpunit/Traits/Steps/StepDatabaseTrait.php (0 hunks)
  • .vortex/tests/phpunit/Traits/Steps/StepPrepareSutTrait.php (3 hunks)
💤 Files with no reviewable changes (3)
  • .vortex/tests/phpunit/Functional/WorkflowTest.php
  • .vortex/tests/phpunit/Traits/Steps/StepDatabaseTrait.php
  • .vortex/tests/phpunit/Functional/WorkflowUtilitiesTest.php
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-08T12:02:24.652Z
Learnt from: AlexSkrypnyk
PR: drevops/vortex#1896
File: .vortex/tests/bats/unit/download-db-lagoon.bats:24-25
Timestamp: 2025-08-08T12:02:24.652Z
Learning: In .vortex/tests/bats/unit Bats tests using ../_helper.bash (run_steps), prefixing a STEPS entry with "- " denotes a negative assertion (the substring must NOT appear in output). Unprefixed entries are positive assertions. Example: "- Database dump refresh requested. Will create a new dump." asserts absence; "Database dump refresh requested. Will create a new dump." asserts presence.

Applied to files:

  • .vortex/tests/README.md
  • .vortex/tests/phpunit/Traits/Steps/StepPrepareSutTrait.php
🧬 Code graph analysis (7)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (1)
.vortex/installer/src/Utils/File.php (1)
  • File (10-121)
.vortex/tests/phpunit/Functional/InstallDbTest.php (1)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (2)
  • FunctionalTestCase (25-263)
  • substepDownloadDb (240-261)
.vortex/tests/phpunit/Functional/DoctorTest.php (1)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (2)
  • FunctionalTestCase (25-263)
  • cmd (104-126)
.vortex/tests/phpunit/Functional/DockerComposeTest.php (2)
.vortex/installer/src/Utils/File.php (1)
  • File (10-121)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (5)
  • FunctionalTestCase (25-263)
  • cmd (104-126)
  • syncToHost (152-159)
  • substepDownloadDb (240-261)
  • cmdFail (128-150)
.vortex/tests/phpunit/Functional/IdempotenceTest.php (5)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (2)
  • FunctionalTestCase (25-263)
  • substepDownloadDb (240-261)
.vortex/tests/phpunit/Traits/Steps/StepBuildTrait.php (1)
  • stepBuild (12-51)
.vortex/tests/phpunit/Traits/AssertProjectFilesTrait.php (1)
  • assertFilesTrackedInGit (293-322)
.vortex/tests/phpunit/Traits/Steps/StepTestTrait.php (1)
  • stepAhoyTestBddFast (109-134)
.vortex/tests/phpunit/Functional/WorkflowTest.php (1)
  • testIdempotence (33-45)
.vortex/tests/phpunit/Functional/AhoyTest.php (2)
.vortex/installer/src/Utils/File.php (1)
  • File (10-121)
.vortex/tests/phpunit/Functional/FunctionalTestCase.php (3)
  • FunctionalTestCase (25-263)
  • cmd (104-126)
  • cmdFail (128-150)
.vortex/tests/phpunit/Traits/Steps/StepPrepareSutTrait.php (1)
.vortex/installer/src/Utils/File.php (1)
  • File (10-121)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
  • GitHub Check: build (0)
  • GitHub Check: build (1)
  • GitHub Check: vortex-test-deployment (0)
  • GitHub Check: vortex-test-deployment (1)
  • GitHub Check: vortex-test-workflow (0)
  • GitHub Check: vortex-test-workflow (4)
  • GitHub Check: vortex-test-workflow (2)
  • GitHub Check: vortex-test-workflow (3)
  • GitHub Check: vortex-test-common
  • GitHub Check: vortex-test-workflow (1)
  • GitHub Check: vortex-test-installer (8.4)
  • GitHub Check: vortex-test-installer (8.3)
  • GitHub Check: vortex-test-installer (8.2)
🔇 Additional comments (9)
.vortex/tests/README.md (1)

55-56: README example update LGTM

Switching to IdempotenceTest reflects the new suite split.

.vortex/tests/phpcs.xml (1)

25-31: PHPCS rule addition LGTM

Allowing missing class comments in tests is reasonable and mirrors existing function‑comment exclusions.

.vortex/tests/phpunit/Functional/FunctionalTestCase.php (1)

240-261: New DB substep is sound

Clear pre/post assertions, env override, and optional container copy when volumes aren’t mounted. No blockers.

.vortex/tests/phpunit/Functional/InstallDbTest.php (1)

18-32: Rename and setup change LGTM

Using substepDownloadDb() in setUp() matches the refactor away from StepDatabaseTrait::stepDownloadDb().

.vortex/tests/phpunit/Traits/Steps/StepPrepareSutTrait.php (1)

76-80: Good hardening

Asserting removal of special comment tokens (‘#;’, ‘#;<’, ‘#;>’) post‑install is a useful safety net.

.vortex/tests/phpunit/Functional/IdempotenceTest.php (1)

12-27: Idempotence flow LGTM

Download DB, build, assert git state, run BDD, rebuild, re‑assert: clear and aligned with new helpers.

.vortex/tests/phpunit/Functional/AhoyTest.php (1)

14-51: Ahoy local workflow tests LGTM

Covers absence/presence and non‑zero exit. Nice use of cmd/cmdFail and File utilities.

.vortex/tests/phpunit/Functional/DoctorTest.php (1)

9-18: Strengthen assertions: ensure ALL expected sections are present

assertProcessAnyOutputContainsOrNot not found in the repository; either replace the helper with explicit per-string assertions in this test or confirm the helper enforces "all" matches.

Location: .vortex/tests/phpunit/Functional/DoctorTest.php (testInfo)

-  public function testInfo(): void {
-    $this->cmd('ahoy doctor info', [
-      'System information report',
-      'OPERATING SYSTEM',
-      'DOCKER',
-      'DOCKER COMPOSE',
-      'PYGMY',
-      'AHOY',
-    ]);
-  }
+  public function testInfo(): void {
+    $process = $this->cmd('ahoy doctor info', txt: 'Verify doctor info contains expected sections');
+    $out = ($process?->getOutput() ?? '') . ($process?->getErrorOutput() ?? '');
+    foreach (['System information report', 'OPERATING SYSTEM', 'DOCKER', 'DOCKER COMPOSE', 'PYGMY', 'AHOY'] as $needle) {
+      $this->assertStringContainsString($needle, $out);
+    }
+  }
.vortex/tests/phpunit/Functional/DockerComposeTest.php (1)

11-16: Good end-to-end workflow coverage and sensible timeouts

End-to-end flow (build → up → deps → lint → DB → provision → tests) is clear and the step timeouts are reasonable. Using substeps and FunctionalTestCase helpers improves readability.

Also applies to: 32-40

Comment on lines +14 to +19
$this->cmd('docker compose up -d --force-recreate', txt: 'Start stack', tio: 15 * 60);
$this->syncToHost();

$this->logSubstep('Installing development dependencies');
$this->cmd('docker compose exec -T cli composer install --prefer-dist', txt: 'Install development dependencies with Composer', tio: 10 * 60);
$this->cmd('docker compose exec -T cli bash -cl "yarn install --frozen-lockfile"', txt: 'Install development dependencies with Yarn', tio: 10 * 60);
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Optional: gate on container readiness before execs

Consider waiting for the cli service to be healthy before running composer/yarn to reduce flakiness in CI.

For example: docker compose ps --format '{{.Service}} {{.Health}}' | grep -q 'cli healthy' (or a small retry loop) before the first exec.

🤖 Prompt for AI Agents
In .vortex/tests/phpunit/Functional/DockerComposeTest.php around lines 14 to 19,
the test runs docker compose exec commands against the cli service immediately
after bringing the stack up which can cause flakiness; add a readiness gate
before the first exec (after docker compose up and syncToHost) that polls docker
compose ps (or uses a small retry loop) until the cli service reports healthy
(e.g. using docker compose ps --format '{{.Service}} {{.Health}}' | grep -q 'cli
healthy') or a timeout is reached; only proceed to run composer and yarn execs
after the readiness check succeeds to reduce CI flakiness.


$this->logSubstep('Installing development dependencies');
$this->cmd('docker compose exec -T cli composer install --prefer-dist', txt: 'Install development dependencies with Composer', tio: 10 * 60);
$this->cmd('docker compose exec -T cli bash -cl "yarn install --frozen-lockfile"', txt: 'Install development dependencies with Yarn', tio: 10 * 60);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Standardize to bash -lc

Minor, but bash -lc is the conventional order; keep it consistent with the fix above.

-    $this->cmd('docker compose exec -T cli bash -cl "yarn install --frozen-lockfile"', txt: 'Install development dependencies with Yarn', tio: 10 * 60);
+    $this->cmd('docker compose exec -T cli bash -lc "yarn install --frozen-lockfile"', txt: 'Install development dependencies with Yarn', tio: 10 * 60);
📝 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
$this->cmd('docker compose exec -T cli bash -cl "yarn install --frozen-lockfile"', txt: 'Install development dependencies with Yarn', tio: 10 * 60);
$this->cmd('docker compose exec -T cli bash -lc "yarn install --frozen-lockfile"', txt: 'Install development dependencies with Yarn', tio: 10 * 60);
🤖 Prompt for AI Agents
In .vortex/tests/phpunit/Functional/DockerComposeTest.php around line 19, the
command uses the unconventional option order "bash -cl"; change it to the
standard order "bash -lc" so the shell options are in canonical order (replace
"-cl" with "-lc" in the docker compose exec command).

Comment on lines +23 to +24
$this->cmd('docker compose exec -T cli vendor/bin/phpstan', txt: 'Lint code with PHPStan', tio: 10 * 60);
$this->cmd('docker compose exec -T cli vendor/bin/rector', txt: 'Lint code with Rector', tio: 10 * 60);
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Be explicit with tool subcommands to avoid defaults changing

phpstan usually needs analyse, and rector should run in --dry-run to avoid modifying code during tests.

-    $this->cmd('docker compose exec -T cli vendor/bin/phpstan', txt: 'Lint code with PHPStan', tio: 10 * 60);
-    $this->cmd('docker compose exec -T cli vendor/bin/rector', txt: 'Lint code with Rector', tio: 10 * 60);
+    $this->cmd('docker compose exec -T cli vendor/bin/phpstan analyse', txt: 'Lint code with PHPStan', tio: 10 * 60);
+    $this->cmd('docker compose exec -T cli vendor/bin/rector --dry-run', txt: 'Lint code with Rector', tio: 10 * 60);
📝 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
$this->cmd('docker compose exec -T cli vendor/bin/phpstan', txt: 'Lint code with PHPStan', tio: 10 * 60);
$this->cmd('docker compose exec -T cli vendor/bin/rector', txt: 'Lint code with Rector', tio: 10 * 60);
$this->cmd('docker compose exec -T cli vendor/bin/phpstan analyse', txt: 'Lint code with PHPStan', tio: 10 * 60);
$this->cmd('docker compose exec -T cli vendor/bin/rector --dry-run', txt: 'Lint code with Rector', tio: 10 * 60);
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/DockerComposeTest.php around lines 23 to 24:
the test shells invoke the tools without explicit subcommands so they may use
changing defaults; update the commands to call phpstan with the 'analyse'
subcommand and run rector with '--dry-run' (and any existing flags) so phpstan
performs analysis explicitly and rector does not modify code during tests.

$this->logSubstep('Linting front-end code');
$this->cmd('docker compose exec -T cli vendor/bin/twig-cs-fixer lint', txt: 'Lint code with TwigCS', tio: 10 * 60);
$this->cmd('docker compose exec -T cli yarn run lint', txt: 'Lint code with module linters', tio: 10 * 60);
$this->cmd('docker compose exec -T cli bash -cl "yarn run --cwd=\${WEBROOT}/themes/custom/\${DRUPAL_THEME} lint"', txt: 'Lint code with theme linters', tio: 10 * 60);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix env var expansion in theme lint command

The escaped dollars prevent expansion of WEBROOT/DRUPAL_THEME inside the container. Also, prefer bash -lc for clarity.

Apply this diff:

-    $this->cmd('docker compose exec -T cli bash -cl "yarn run --cwd=\${WEBROOT}/themes/custom/\${DRUPAL_THEME} lint"', txt: 'Lint code with theme linters', tio: 10 * 60);
+    $this->cmd('docker compose exec -T cli bash -lc "yarn run --cwd=${WEBROOT}/themes/custom/${DRUPAL_THEME} lint"', txt: 'Lint code with theme linters', tio: 10 * 60);
📝 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
$this->cmd('docker compose exec -T cli bash -cl "yarn run --cwd=\${WEBROOT}/themes/custom/\${DRUPAL_THEME} lint"', txt: 'Lint code with theme linters', tio: 10 * 60);
$this->cmd('docker compose exec -T cli bash -lc "yarn run --cwd=${WEBROOT}/themes/custom/${DRUPAL_THEME} lint"', txt: 'Lint code with theme linters', tio: 10 * 60);
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/DockerComposeTest.php around line 30: the
command currently escapes the $ signs so WEBROOT and DRUPAL_THEME are not
expanded inside the container and uses `bash -cl`; update the command invocation
to allow env var expansion by removing the backslashes before the dollar signs
(so ${WEBROOT}/${DRUPAL_THEME} expands in the container) and change the shell
flag to `bash -lc` for clarity; adjust the surrounding quoting as needed so the
full command remains a single string argument to docker compose exec.

Comment on lines +49 to +51
$package_token = getenv('TEST_PACKAGE_TOKEN');
$this->assertNotEmpty($package_token, 'TEST_PACKAGE_TOKEN environment variable must be set');

Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Skip when token is absent to keep local runs green

Hard‑failing when TEST_PACKAGE_TOKEN is unset makes local runs flaky. Skip instead.

-    $package_token = getenv('TEST_PACKAGE_TOKEN');
-    $this->assertNotEmpty($package_token, 'TEST_PACKAGE_TOKEN environment variable must be set');
+    $package_token = getenv('TEST_PACKAGE_TOKEN');
+    if (empty($package_token)) {
+      $this->markTestSkipped('TEST_PACKAGE_TOKEN is not set; skipping token build test.');
+    }
📝 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
$package_token = getenv('TEST_PACKAGE_TOKEN');
$this->assertNotEmpty($package_token, 'TEST_PACKAGE_TOKEN environment variable must be set');
$package_token = getenv('TEST_PACKAGE_TOKEN');
if (empty($package_token)) {
$this->markTestSkipped('TEST_PACKAGE_TOKEN is not set; skipping token build test.');
}
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/DockerComposeTest.php around lines 49-51: the
test currently hard-fails with assertNotEmpty when TEST_PACKAGE_TOKEN is unset;
change this to skip the test when the env var is missing by checking
getenv('TEST_PACKAGE_TOKEN') and calling
$this->markTestSkipped('TEST_PACKAGE_TOKEN environment variable must be set') if
empty, so local runs remain green, then proceed only when the token is present.

Comment on lines +52 to +62
$this->logSubstep('Adding private package to test GitHub token');
File::remove('composer.lock');
$this->cmd('composer config repositories.test-private-package vcs git@github.com:drevops/test-private-package.git');
$this->cmd('composer require --no-update drevops/test-private-package:^1');

$this->logSubstep('Building without PACKAGE_TOKEN - should fail');
$this->cmdFail('docker compose build cil --no-cache', txt: 'Build stack images without token', tio: 15 * 60);

$this->logSubstep('Building with PACKAGE_TOKEN - should succeed');
$this->cmd('docker compose build cli --no-cache', txt: 'Build stack images with token', env: ['PACKAGE_TOKEN' => $package_token], tio: 15 * 60);
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Hermeticity + service typo: wrap repo edits in try/finally and build the correct service

  • Edits to composer.json/composer.lock should be reverted to avoid polluting subsequent tests.
  • Build target should be cli (typo cil currently causes a false failure unrelated to the token).
-    $this->logSubstep('Adding private package to test GitHub token');
-    File::remove('composer.lock');
-    $this->cmd('composer config repositories.test-private-package vcs git@github.com:drevops/test-private-package.git');
-    $this->cmd('composer require --no-update drevops/test-private-package:^1');
-
-    $this->logSubstep('Building without PACKAGE_TOKEN - should fail');
-    $this->cmdFail('docker compose build cil --no-cache', txt: 'Build stack images without token', tio: 15 * 60);
-
-    $this->logSubstep('Building with PACKAGE_TOKEN - should succeed');
-    $this->cmd('docker compose build cli --no-cache', txt: 'Build stack images with token', env: ['PACKAGE_TOKEN' => $package_token], tio: 15 * 60);
+    $this->backupFile('composer.json');
+    try {
+      $this->logSubstep('Adding private package to test GitHub token');
+      File::remove('composer.lock');
+      $this->cmd('composer config repositories.test-private-package vcs git@github.com:drevops/test-private-package.git');
+      $this->cmd('composer require --no-update drevops/test-private-package:^1');
+
+      $this->logSubstep('Building without PACKAGE_TOKEN - should fail');
+      $this->cmdFail('docker compose build cli --no-cache', txt: 'Build stack images without token', tio: 15 * 60);
+
+      $this->logSubstep('Building with PACKAGE_TOKEN - should succeed');
+      $this->cmd('docker compose build cli --no-cache', txt: 'Build stack images with token', env: ['PACKAGE_TOKEN' => $package_token], tio: 15 * 60);
+    }
+    finally {
+      $this->restoreFile('composer.json');
+      File::remove('composer.lock');
+    }
📝 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
$this->logSubstep('Adding private package to test GitHub token');
File::remove('composer.lock');
$this->cmd('composer config repositories.test-private-package vcs git@github.com:drevops/test-private-package.git');
$this->cmd('composer require --no-update drevops/test-private-package:^1');
$this->logSubstep('Building without PACKAGE_TOKEN - should fail');
$this->cmdFail('docker compose build cil --no-cache', txt: 'Build stack images without token', tio: 15 * 60);
$this->logSubstep('Building with PACKAGE_TOKEN - should succeed');
$this->cmd('docker compose build cli --no-cache', txt: 'Build stack images with token', env: ['PACKAGE_TOKEN' => $package_token], tio: 15 * 60);
}
$this->backupFile('composer.json');
try {
$this->logSubstep('Adding private package to test GitHub token');
File::remove('composer.lock');
$this->cmd('composer config repositories.test-private-package vcs git@github.com:drevops/test-private-package.git');
$this->cmd('composer require --no-update drevops/test-private-package:^1');
$this->logSubstep('Building without PACKAGE_TOKEN - should fail');
$this->cmdFail('docker compose build cli --no-cache', txt: 'Build stack images without token', tio: 15 * 60);
$this->logSubstep('Building with PACKAGE_TOKEN - should succeed');
$this->cmd('docker compose build cli --no-cache', txt: 'Build stack images with token', env: ['PACKAGE_TOKEN' => $package_token], tio: 15 * 60);
}
finally {
$this->restoreFile('composer.json');
File::remove('composer.lock');
}
}
🤖 Prompt for AI Agents
.vortex/tests/phpunit/Functional/DockerComposeTest.php lines 52-62: the test
mutates composer.json/composer.lock and has a typo building service "cil" which
causes spurious failures; wrap the composer repository edits and composer.lock
removal in a try/finally (or equivalent) to ensure original composer.json and
composer.lock are restored after the test (use backups or revert changes in
finally), and fix the build command typo by changing 'docker compose build cil'
to 'docker compose build cli' in the failing build assertion so the test
exercises the intended service.

@codecov
Copy link

codecov bot commented Sep 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.23%. Comparing base (367866e) to head (c50923f).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2017      +/-   ##
===========================================
+ Coverage    70.34%   74.23%   +3.89%     
===========================================
  Files           89       89              
  Lines         5543     5543              
  Branches        44       44              
===========================================
+ Hits          3899     4115     +216     
+ Misses        1644     1428     -216     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AlexSkrypnyk AlexSkrypnyk merged commit 76098d4 into develop Sep 15, 2025
31 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/bats2phpunit-4 branch September 15, 2025 06:12
@github-project-automation github-project-automation bot moved this from BACKLOG to Release queue in Vortex Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Released in 25.10.0

Development

Successfully merging this pull request may close these issues.

2 participants