From 530356eaf6e369fa28a4d0bacdb29aca1cbe3b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sun, 5 Apr 2026 22:32:24 +0200 Subject: [PATCH 1/3] Add test helper methods to RequestConverterTest - Add setUp/tearDown for automatic temp file cleanup - Add createTempFile() helper method - Add createRequestWithFiles() helper method - Refactor testMalformedFileDataThrowsException to use helpers Reduces test boilerplate from ~30 lines to ~15 lines per test. (#88) --- CHANGELOG.md | 6 +++ tests/RequestConverterTest.php | 70 +++++++++++++++++++++------------- 2 files changed, 50 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b14232c..1ed2ed6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Added test helper methods in `RequestConverterTest` for temp file cleanup and request creation ([#88](https://github.com/crazy-goat/workerman-bundle/issues/88)) + - Added `setUp()`/`tearDown()` for automatic temp file cleanup + - Added `createTempFile()` helper method + - Added `createRequestWithFiles()` helper method + - Reduces test boilerplate and improves readability + - Added `QUERY_STRING` to server bag in `RequestConverter` ([#66](https://github.com/crazy-goat/workerman-bundle/issues/66)) - Enables `$request->server->get('QUERY_STRING')` to return query string - Enables Symfony's `getQueryString()` to work correctly diff --git a/tests/RequestConverterTest.php b/tests/RequestConverterTest.php index 04a2604..9712a46 100644 --- a/tests/RequestConverterTest.php +++ b/tests/RequestConverterTest.php @@ -14,6 +14,18 @@ */ final class RequestConverterTest extends TestCase { + /** @var string[] */ + private array $tempFiles = []; + + protected function tearDown(): void + { + foreach ($this->tempFiles as $file) { + @unlink($file); + } + $this->tempFiles = []; + + parent::tearDown(); + } public function testValidFileStructureIsAccepted(): void { $buffer = $this->createMultipartRequest( @@ -116,37 +128,17 @@ public function testMalformedFileDataThrowsException(): void $this->expectException(InvalidArgumentException::class); $this->expectExceptionMessage('missing required field'); - // Create a temp file so Workerman doesn't clear our files array - $tmpFile = tempnam(sys_get_temp_dir(), 'test_'); - file_put_contents($tmpFile, 'test content'); + $tmpFile = $this->createTempFile('test content'); $buffer = "POST /test HTTP/1.1\r\nHost: localhost\r\n\r\n"; - $rawRequest = new Request($buffer); - - // Manually inject malformed file data (missing required fields) but with valid tmp_name - // so Workerman's file() method doesn't clear it - $reflection = new \ReflectionClass($rawRequest); - $dataProperty = $reflection->getProperty('data'); - $dataProperty->setValue($rawRequest, [ - 'files' => [ - 'malformed_file' => [ - 'name' => 'test.txt', - 'tmp_name' => $tmpFile, - // Missing 'type', 'size', 'error' - should trigger validation error - ], + $rawRequest = $this->createRequestWithFiles($buffer, [ + 'malformed_file' => [ + 'name' => 'test.txt', + 'tmp_name' => $tmpFile, ], ]); - try { - // This should throw InvalidArgumentException - RequestConverter::toSymfonyRequest($rawRequest); - // If we get here, delete the file - unlink($tmpFile); - } catch (InvalidArgumentException $e) { - // Exception thrown as expected, clean up - unlink($tmpFile); - throw $e; - } + RequestConverter::toSymfonyRequest($rawRequest); } public function testHeadersAreAvailableInServerBag(): void @@ -425,4 +417,30 @@ private function createMultipartRequest(string $boundary, array $fields): string return $buffer . $body; } + + private function createTempFile(string $content = 'test'): string + { + $tmpFile = tempnam(sys_get_temp_dir(), 'test_'); + if ($tmpFile === false) { + throw new \RuntimeException('Failed to create temp file'); + } + file_put_contents($tmpFile, $content); + $this->tempFiles[] = $tmpFile; + + return $tmpFile; + } + + /** + * @param array> $files + */ + private function createRequestWithFiles(string $buffer, array $files): Request + { + $rawRequest = new Request($buffer); + + $reflection = new \ReflectionClass($rawRequest); + $dataProperty = $reflection->getProperty('data'); + $dataProperty->setValue($rawRequest, ['files' => $files]); + + return $rawRequest; + } } From a5e7a09e602a936c906400d643fc494ba54473c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sun, 5 Apr 2026 22:37:31 +0200 Subject: [PATCH 2/3] Fix code review issues: CHANGELOG, file_put_contents guard, blank line, reflection comment --- CHANGELOG.md | 2 +- tests/RequestConverterTest.php | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ed2ed6..d7ed23b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added test helper methods in `RequestConverterTest` for temp file cleanup and request creation ([#88](https://github.com/crazy-goat/workerman-bundle/issues/88)) - - Added `setUp()`/`tearDown()` for automatic temp file cleanup + - Added `tearDown()` for automatic temp file cleanup - Added `createTempFile()` helper method - Added `createRequestWithFiles()` helper method - Reduces test boilerplate and improves readability diff --git a/tests/RequestConverterTest.php b/tests/RequestConverterTest.php index 9712a46..1d1b0e0 100644 --- a/tests/RequestConverterTest.php +++ b/tests/RequestConverterTest.php @@ -26,6 +26,7 @@ protected function tearDown(): void parent::tearDown(); } + public function testValidFileStructureIsAccepted(): void { $buffer = $this->createMultipartRequest( @@ -424,7 +425,9 @@ private function createTempFile(string $content = 'test'): string if ($tmpFile === false) { throw new \RuntimeException('Failed to create temp file'); } - file_put_contents($tmpFile, $content); + if (file_put_contents($tmpFile, $content) === false) { + throw new \RuntimeException('Failed to write to temp file'); + } $this->tempFiles[] = $tmpFile; return $tmpFile; @@ -437,6 +440,8 @@ private function createRequestWithFiles(string $buffer, array $files): Request { $rawRequest = new Request($buffer); + // Workerman's Request does not expose a public API for file injection. + // Reflection is required to simulate malformed file uploads for testing. $reflection = new \ReflectionClass($rawRequest); $dataProperty = $reflection->getProperty('data'); $dataProperty->setValue($rawRequest, ['files' => $files]); From 33119189881b78e453a49fc98eacb835a73fe265 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Sun, 5 Apr 2026 22:45:15 +0200 Subject: [PATCH 3/3] Prepare CHANGELOG for v0.13.0 release --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d7ed23b..db4bc33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.13.0] - 2026-04-05 + ### Added - Added test helper methods in `RequestConverterTest` for temp file cleanup and request creation ([#88](https://github.com/crazy-goat/workerman-bundle/issues/88))