[#1976] Upload SUT test artifacts in vortex-test-workflow CI #2255
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughAdds collection of SUT test logs during PHPUnit tearDown and a GitHub Actions step to always upload collected test artifacts (including hidden files) from CI after tests run. Changes
Sequence Diagram(s)sequenceDiagram
participant PHPUnit as "PHPUnit Test"
participant SUT as "SUT Container"
participant Host as "Host FS (.vortex/tests/.logs)"
participant Actions as "GitHub Actions (upload-artifact)"
PHPUnit->>SUT: run test (may produce .logs)
PHPUnit->>PHPUnit: tearDown() calls collectSutArtifacts(test_failed)
alt test_failed
PHPUnit->>SUT: try sync `.logs` -> Host (errors ignored)
end
SUT-->>Host: `.logs` available on host
PHPUnit->>Host: copy `.logs` -> `.logs/sut/{test_id}`
Actions->>Host: workflow step always runs
Actions->>Actions: upload `.vortex/tests/.logs` (include hidden files)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.vortex/tests/phpunit/Functional/FunctionalTestCase.php:
- Around line 102-103: The current artifact destination uses only $this->name(),
risking collisions and unsafe characters; update the destination construction in
FunctionalTestCase so it includes the test class name (e.g., get_class($this) or
static::class) and a sanitized test identifier, then ensure uniqueness (append a
timestamp or uniqid()) and strip or replace filesystem-unsafe characters
(spaces, slashes, colons, non-alphanumerics) before calling File::copy with the
resulting $dest. Target the code that sets $dest and the call to File::copy to
perform the sanitization and uniqueness logic.
Member
Author
|
This comment has been minimized.
This comment has been minimized.
…on and uniqueness.
|
Member
Author
|
Member
Author
|
Member
Author
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2255 +/- ##
==========================================
- Coverage 76.85% 76.22% -0.63%
==========================================
Files 112 105 -7
Lines 5846 5687 -159
Branches 44 0 -44
==========================================
- Hits 4493 4335 -158
+ Misses 1353 1352 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.vortex/tests/phpunit/Functional/FunctionalTestCase.php:
- Around line 102-109: In tearDown() where $dest is built and
File::copy($sut_logs, $dest) is called, ensure the destination directory exists
(create dirname($dest) with appropriate permissions) before copying, and wrap
the File::copy call in a try/catch that catches the FileException (or
\Exception) so copy failures are logged via $this->logNote or a warning but not
rethrown (so they don't mask test results); reference the $dest variable,
File::copy, the surrounding tearDown() method and $this->logNote when making
this change.
Comment on lines
+102
to
+109
| $class_name = (new \ReflectionClass($this))->getShortName(); | ||
| $test_id = $class_name . '_' . $this->name() . '_' . uniqid(); | ||
| $test_id = (string) preg_replace('/[^a-zA-Z0-9_-]/', '_', $test_id); | ||
|
|
||
| $dest = dirname(__DIR__, 2) . '/.logs/sut/' . $test_id; | ||
| File::copy($sut_logs, $dest); | ||
|
|
||
| $this->logNote('SUT test artifacts copied to: ' . $dest); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
# Find the File class being used
rg 'class File' --type php -A 5 | head -50Repository: drevops/vortex
Length of output: 40
🏁 Script executed:
# Look for File class imports or usage
rg 'use.*File' .vortex/tests/phpunit/Functional/FunctionalTestCase.phpRepository: drevops/vortex
Length of output: 85
🏁 Script executed:
# Search for File::copy definition
rg 'function copy\(' --type phpRepository: drevops/vortex
Length of output: 40
🏁 Script executed:
# Check if File class is from a composer package
cat composer.json | jq '.require, .require-dev' 2>/dev/null || find . -name "composer.json" -type f -exec cat {} \; | head -100Repository: drevops/vortex
Length of output: 4691
🌐 Web query:
alexskrypnyk/file 0.19.0 File::copy() documentation
💡 Result:
For alexskrypnyk/file v0.19.0 (released 2025-12-23), the README documents AlexSkrypnyk\File\File::copy() as a static utility to copy a file or a directory (the example shows copying a directory recursively). It’s intended to be used inside a try/catch for FileException. [1]
Example (from the v0.19.0 README):
use AlexSkrypnyk\File\Exception\FileException;
use AlexSkrypnyk\File\File;
try {
File::copy('/path/to/source', '/path/to/destination');
} catch (FileException $e) {
echo $e->getMessage();
}
``` [1]
**Where it’s listed:**
- “Available Methods” table: **`copy()` — Copy file or directory.** [1]
**Source:** Packagist README for `alexskrypnyk/file` **0.19.0**. [1]
References:
[1] https://packagist.org/packages/alexskrypnyk/file
---
🌐 Web query:
`github AlexSkrypnyk file php copy method parent directories`
💡 Result:
I couldn’t find a specific `File.php` (or a `copy()` implementation) in an `AlexSkrypnyk` GitHub repo from that query, but in plain PHP the key point is: **`copy($from, $to)` will fail if the destination directory doesn’t exist**—it does not create parent directories for you. [1]
Create the parent dirs first (recursively), then copy:
```php
$from = $src;
$to = $dst;
$dir = dirname($to);
if (!is_dir($dir)) {
mkdir($dir, 0777, true); // recursive parent directories
}
if (!copy($from, $to)) {
throw new RuntimeException("Failed to copy $from to $to");
}mkdir(..., $recursive=true)creates nested/parent directories. [2]copy()copies/overwrites the destination file, but won’t create missing directories. [1]
Sources: [1] [2]
Defensive directory creation + copy error handling.
The .logs/sut directory must be created explicitly before copying, and File::copy() throws FileException on failure. In tearDown(), artifact logging failures should not mask test results. Ensure the destination directory exists and handle copy exceptions gracefully.
♻️ Suggested hardening
- $dest = dirname(__DIR__, 2) . '/.logs/sut/' . $test_id;
- File::copy($sut_logs, $dest);
+ $dest_root = dirname(__DIR__, 2) . '/.logs/sut';
+ if (!is_dir($dest_root) && !mkdir($dest_root, 0777, TRUE) && !is_dir($dest_root)) {
+ $this->logNote('Unable to create SUT artifacts directory: ' . $dest_root);
+ return;
+ }
+ $dest = $dest_root . '/' . $test_id;
+ try {
+ File::copy($sut_logs, $dest);
+ }
+ catch (\Throwable $e) {
+ $this->logNote('Failed to copy SUT artifacts: ' . $e->getMessage());
+ return;
+ }📝 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
| $class_name = (new \ReflectionClass($this))->getShortName(); | |
| $test_id = $class_name . '_' . $this->name() . '_' . uniqid(); | |
| $test_id = (string) preg_replace('/[^a-zA-Z0-9_-]/', '_', $test_id); | |
| $dest = dirname(__DIR__, 2) . '/.logs/sut/' . $test_id; | |
| File::copy($sut_logs, $dest); | |
| $this->logNote('SUT test artifacts copied to: ' . $dest); | |
| $class_name = (new \ReflectionClass($this))->getShortName(); | |
| $test_id = $class_name . '_' . $this->name() . '_' . uniqid(); | |
| $test_id = (string) preg_replace('/[^a-zA-Z0-9_-]/', '_', $test_id); | |
| $dest_root = dirname(__DIR__, 2) . '/.logs/sut'; | |
| if (!is_dir($dest_root) && !mkdir($dest_root, 0777, TRUE) && !is_dir($dest_root)) { | |
| $this->logNote('Unable to create SUT artifacts directory: ' . $dest_root); | |
| return; | |
| } | |
| $dest = $dest_root . '/' . $test_id; | |
| try { | |
| File::copy($sut_logs, $dest); | |
| } | |
| catch (\Throwable $e) { | |
| $this->logNote('Failed to copy SUT artifacts: ' . $e->getMessage()); | |
| return; | |
| } | |
| $this->logNote('SUT test artifacts copied to: ' . $dest); |
🤖 Prompt for AI Agents
In @.vortex/tests/phpunit/Functional/FunctionalTestCase.php around lines 102 -
109, In tearDown() where $dest is built and File::copy($sut_logs, $dest) is
called, ensure the destination directory exists (create dirname($dest) with
appropriate permissions) before copying, and wrap the File::copy call in a
try/catch that catches the FileException (or \Exception) so copy failures are
logged via $this->logNote or a warning but not rethrown (so they don't mask test
results); reference the $dest variable, File::copy, the surrounding tearDown()
method and $this->logNote when making this change.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
collectSutArtifacts()method toFunctionalTestCase::tearDown()that copies SUT.logs/to.vortex/tests/.logs/sut/{testName}/before workspace cleanupsyncToHost(), then copies to the persistent locationif: always()to thevortex-test-workflowCI job to upload.vortex/tests/.logs/(includes both PHPUnit runner'sjunit.xmland SUT artifacts)Closes #1976
Test plan
sut/{testName}/if-no-files-found: ignore)Summary by CodeRabbit