Skip to content

Conversation

@AlexSkrypnyk
Copy link
Member

@AlexSkrypnyk AlexSkrypnyk commented Dec 2, 2025

Summary by CodeRabbit

  • Tests
    • Refactored test infrastructure to use centralized helper methods for managing environment variables in place of direct system calls.
    • Added automated environment cleanup after test execution to ensure proper variable isolation.
    • New validation tests verify environment variable isolation between tests.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 2, 2025

Walkthrough

This pull request refactors test environment variable management across the test suite by replacing direct putenv() calls with abstracted static helper methods (envSet, envUnset, envSetMultiple, envUnsetPrefix). The UnitTestCase class now includes EnvTrait with automatic cleanup via envReset() in tearDown(), and new cleanup verification tests are added to SelfTest.

Changes

Cohort / File(s) Change Summary
Test Environment Variable Abstraction
.vortex/installer/tests/Unit/ConfigTest.php, .vortex/installer/tests/Unit/DownloaderTest.php, .vortex/installer/tests/Unit/EnvTest.php, .vortex/installer/tests/Unit/TuiTest.php
Replaced direct putenv() calls with static helper methods (envSet, envUnset, envSetMultiple, envUnsetPrefix) for environment variable setup and removed explicit cleanup code.
Test Framework Setup
.vortex/installer/tests/Unit/UnitTestCase.php
Added EnvTrait import and usage; introduced tearDown() method override that calls static::envReset() before delegating to parent.
New Cleanup Verification Tests
.vortex/installer/tests/Unit/SelfTest.php
Added testEnvCleanup1SetVariables() and testEnvCleanup2VerifyCleanup() methods with Depends attribute to verify environment variable cleanup between tests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Homogeneous refactoring pattern applied consistently across multiple test files
  • All changes are test-only with no production code logic modifications
  • Primary effort is verifying the helper method substitutions are correct and complete
  • New test methods in SelfTest.php follow the established pattern and are straightforward

Possibly related PRs

Suggested labels

AUTOMERGE

Poem

🐰 From putenv's chaos, helpers rise,
Environment vars now organized and wise,
Tests cleanup neatly, setup so clean,
Abstraction's the best we've ever seen! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 7.14% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: refactoring installer tests to use envTrait for automated environment variable cleanup instead of manual putenv handling.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/update-env-installer

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 541ff54 and 01a770a.

📒 Files selected for processing (6)
  • .vortex/installer/tests/Unit/ConfigTest.php (4 hunks)
  • .vortex/installer/tests/Unit/Downloader/DownloaderTest.php (2 hunks)
  • .vortex/installer/tests/Unit/EnvTest.php (5 hunks)
  • .vortex/installer/tests/Unit/SelfTest.php (2 hunks)
  • .vortex/installer/tests/Unit/TuiTest.php (2 hunks)
  • .vortex/installer/tests/Unit/UnitTestCase.php (3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 0
File: :0-0
Timestamp: 2025-05-29T12:15:32.188Z
Learning: Do not review files in `.vortex/installer/tests/Fixtures/install` directory as they are test fixtures.
📚 Learning: 2025-06-01T08:10:15.903Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 1693
File: .vortex/installer/src/Prompts/Handlers/Internal.php:52-52
Timestamp: 2025-06-01T08:10:15.903Z
Learning: The File utility class in .vortex/installer/src/Utils/File.php extends AlexSkrypnyk\File\File, and methods like collapseRepeatedEmptyLines() are available from this external parent class dependency.

Applied to files:

  • .vortex/installer/tests/Unit/SelfTest.php
  • .vortex/installer/tests/Unit/UnitTestCase.php
📚 Learning: 2025-09-13T04:14:41.765Z
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2011
File: .vortex/tests/phpunit/Traits/Steps/StepAhoyTrait.php:20-23
Timestamp: 2025-09-13T04:14:41.765Z
Learning: In .vortex/tests/phpunit PHPUnit tests using cmd/cmdFail helpers, patterns in the expected output arrays support '*' markers for positive assertions (text must be present) and '!' markers for negative assertions (text must NOT be present). The '*' markers are NOT literal text matches but rather indicate expected presence.

Applied to files:

  • .vortex/installer/tests/Unit/UnitTestCase.php
  • .vortex/installer/tests/Unit/TuiTest.php
🧬 Code graph analysis (2)
.vortex/installer/tests/Unit/ConfigTest.php (1)
.vortex/installer/src/Utils/Config.php (1)
  • Config (14-126)
.vortex/installer/tests/Unit/TuiTest.php (2)
.vortex/installer/src/Utils/Tui.php (2)
  • Tui (15-259)
  • box (172-191)
.vortex/installer/src/Utils/Strings.php (2)
  • Strings (7-192)
  • stripAnsiColors (18-20)
⏰ 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). (11)
  • GitHub Check: build (1)
  • GitHub Check: build (0)
  • GitHub Check: vortex-test-installer (8.3)
  • GitHub Check: vortex-test-installer (8.4)
  • GitHub Check: vortex-test-workflow (4)
  • GitHub Check: vortex-test-workflow (3)
  • GitHub Check: vortex-test-workflow (2)
  • GitHub Check: vortex-test-workflow (0)
  • GitHub Check: vortex-test-workflow (1)
  • GitHub Check: vortex-test-common
  • GitHub Check: vortex-test-docs
🔇 Additional comments (16)
.vortex/installer/tests/Unit/UnitTestCase.php (3)

10-10: LGTM: EnvTrait import added.

The import enables environment helper methods across all unit tests.


29-29: LGTM: EnvTrait usage added.

This provides all test classes with centralized environment management helpers.


44-50: LGTM: Centralized environment cleanup in tearDown.

The override correctly calls envReset() before delegating to the parent, ensuring all environment variables set during tests are automatically cleaned up.

.vortex/installer/tests/Unit/Downloader/DownloaderTest.php (1)

252-252: LGTM: Environment variable setup migrated to helper method.

Both test methods now use static::envSet() for setting GITHUB_TOKEN, with automatic cleanup handled by the base class tearDown.

Also applies to: 278-278

.vortex/installer/tests/Unit/EnvTest.php (4)

51-51: LGTM: Environment variable setup uses helper method.

The change from putenv() to static::envSet() is correct.


69-74: LGTM: Conditional environment setup handles null values.

The conditional logic correctly uses envSet() when a value is expected and envUnset() when null is expected.


99-99: LGTM: Helper method used for environment setup.

Correctly uses static::envSet() instead of direct putenv().


315-315: LGTM: Environment cleanup uses helper method.

Both instances correctly use static::envUnset() instead of putenv($name, false).

Also applies to: 334-334

.vortex/installer/tests/Unit/ConfigTest.php (3)

22-22: LGTM: Simplified environment cleanup using prefix helper.

Using envUnsetPrefix('VORTEX_INSTALLER') is much cleaner than individually unsetting each environment variable.


171-171: LGTM: Environment setup uses helper method.

Both test methods correctly use static::envSet() instead of direct putenv() calls.

Also applies to: 185-185


325-329: LGTM: Bulk environment setup uses helper method.

Using envSetMultiple() to set multiple environment variables at once is more concise than individual putenv() calls.

.vortex/installer/tests/Unit/SelfTest.php (2)

9-9: LGTM: Import added for test dependency.

The Depends attribute enables ordered test execution for cleanup verification.


24-53: LGTM: Excellent validation of environment cleanup.

The two dependent tests effectively demonstrate and verify that:

  • envSet() and envSetMultiple() correctly set environment variables
  • tearDown() automatically cleans up environment variables between tests

This validates the core functionality of the EnvTrait integration.

.vortex/installer/tests/Unit/TuiTest.php (3)

168-171: LGTM: Simplified terminal width mocking.

Using envSet('COLUMNS', ...) eliminates the need for manual backup and restore logic.


173-186: LGTM: Simplified test logic.

The refactoring improves readability while maintaining the same test behavior.


533-546: LGTM: Environment setup simplified with helper methods.

Replacing the try/finally blocks with envSet() and envUnset() removes boilerplate while maintaining test isolation through automatic cleanup.


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

@AlexSkrypnyk AlexSkrypnyk added this to the 25.11.0 milestone Dec 2, 2025
@github-actions github-actions bot temporarily deployed to commit December 2, 2025 03:58 Inactive
@codecov
Copy link

codecov bot commented Dec 2, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.90%. Comparing base (92a94c2) to head (01a770a).
⚠️ Report is 3 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2144      +/-   ##
===========================================
+ Coverage    70.89%   70.90%   +0.01%     
===========================================
  Files           99       99              
  Lines         5075     5077       +2     
  Branches        44       44              
===========================================
+ Hits          3598     3600       +2     
  Misses        1477     1477              

☔ 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 2a20aa3 into develop Dec 2, 2025
28 checks passed
@AlexSkrypnyk AlexSkrypnyk deleted the feature/update-env-installer branch December 2, 2025 04:16
@github-project-automation github-project-automation bot moved this from BACKLOG to Release queue in Vortex Dec 2, 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 1.34.0

Development

Successfully merging this pull request may close these issues.

2 participants