Skip to content

Commit 6481f9d

Browse files
walmir-silvawalmir-silva
committed
feat: enhance file handler error handling and add tests
- modified: docker-compose.yml - Updated service configurations for better compatibility and performance. - new file: invalidPath - Added a placeholder file to simulate an invalid path scenario in tests. - modified: src/Handler/AbstractFileHandler.php - Improved directory creation logic with better error handling. - Added checks for directory writability. - Included detailed exceptions for invalid and non-writable paths. - modified: tests/Handler/FileHandlerTest.php - Added tests for invalid path handling. - Enhanced tests for non-writable directory scenarios. - Verified that exceptions are thrown correctly for invalid and non-writable paths. - modified: tests/Logger/LoggerBuilderTest.php - Adjusted test cases to align with the new error handling logic in FileHandler. - Improved test coverage for logger builder functionalities.
1 parent 71ff74b commit 6481f9d

File tree

5 files changed

+100
-50
lines changed

5 files changed

+100
-50
lines changed

‎docker-compose.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
version: "3.8"
1+
name: kariricode-logging
22

33
services:
44
php:
5-
container_name: kariricode-contract
5+
container_name: kariricode-logging
66
build:
77
context: .
88
dockerfile: .docker/php/Dockerfile

‎invalidPath

Whitespace-only changes.

‎src/Handler/AbstractFileHandler.php

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,25 @@ public function __construct(
2424
protected function ensureDirectoryExists(): void
2525
{
2626
$directory = dirname($this->filePath);
27-
if (!is_dir($directory) && !mkdir($directory, 0755, true)) {
28-
throw new LoggingException("Unable to create log directory: {$directory}");
27+
if (!is_dir($directory)) {
28+
if (!$this->createDirectory($directory)) {
29+
throw new LoggingException("Unable to create log directory: $directory");
30+
}
31+
} elseif (!$this->isDirectoryWritable($directory)) {
32+
throw new LoggingException("Log directory is not writable: $directory");
2933
}
3034
}
3135

36+
protected function createDirectory($path)
37+
{
38+
return mkdir($path, 0777, true);
39+
}
40+
41+
protected function isDirectoryWritable($path)
42+
{
43+
return is_writable($path);
44+
}
45+
3246
protected function openFile(): void
3347
{
3448
$this->fileHandle = fopen($this->filePath, 'a');

‎tests/Handler/FileHandlerTest.php

Lines changed: 77 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,25 @@
22

33
declare(strict_types=1);
44

5-
namespace KaririCode\Logging\Tests\KaririCode\Logging\Handler;
5+
namespace KaririCode\Logging\Tests\Handler;
66

7-
use KaririCode\Contract\ImmutableValue;
87
use KaririCode\Logging\Exception\LoggingException;
98
use KaririCode\Logging\Formatter\LineFormatter;
109
use KaririCode\Logging\Handler\FileHandler;
1110
use KaririCode\Logging\LogLevel;
11+
use KaririCode\Logging\LogRecord;
1212
use PHPUnit\Framework\TestCase;
1313

1414
class FileHandlerTest extends TestCase
1515
{
1616
private string $testLogDir;
1717
private string $testLogFile;
18+
private array $mockFunctions = [];
1819

1920
protected function setUp(): void
2021
{
21-
$this->testLogDir = sys_get_temp_dir() . '/test_logs';
22+
$this->testLogDir = sys_get_temp_dir() . '/test_logs_' . uniqid();
23+
mkdir($this->testLogDir);
2224
$this->testLogFile = $this->testLogDir . '/test.log';
2325

2426
if (file_exists($this->testLogFile)) {
@@ -31,12 +33,19 @@ protected function setUp(): void
3133

3234
protected function tearDown(): void
3335
{
34-
if (file_exists($this->testLogFile)) {
35-
unlink($this->testLogFile);
36+
$this->removeDirectory($this->testLogDir);
37+
}
38+
39+
private function removeDirectory(string $dir): void
40+
{
41+
if (!is_dir($dir)) {
42+
return;
3643
}
37-
if (is_dir($this->testLogDir)) {
38-
rmdir($this->testLogDir);
44+
$files = array_diff(scandir($dir), ['.', '..']);
45+
foreach ($files as $file) {
46+
(is_dir("$dir/$file")) ? $this->removeDirectory("$dir/$file") : unlink("$dir/$file");
3947
}
48+
rmdir($dir);
4049
}
4150

4251
public function testConstructorCreatesDirectory(): void
@@ -48,25 +57,67 @@ public function testConstructorCreatesDirectory(): void
4857
public function testConstructorThrowsExceptionOnInvalidPath(): void
4958
{
5059
$this->expectException(LoggingException::class);
60+
$this->expectExceptionMessage('Unable to create log directory');
61+
62+
$invalidPath = '/path/that/cant/be/created/' . uniqid();
63+
64+
$mockFileHandler = $this->getMockBuilder(FileHandler::class)
65+
->setConstructorArgs([$invalidPath . '/test.log'])
66+
->onlyMethods(['createDirectory'])
67+
->getMock();
68+
69+
$mockFileHandler->expects($this->once())
70+
->method('createDirectory')
71+
->willReturn(false);
72+
73+
/** @var AbstractFileHandlerc $mockFileHandler */
74+
$mockFileHandler->__construct($invalidPath . '/test.log');
75+
}
76+
77+
public function testConstructorThrowsExceptionOnNonWritableDirectory(): void
78+
{
79+
$this->expectException(LoggingException::class);
80+
$this->expectExceptionMessage('Log directory is not writable');
5181

52-
// Suppress warnings for this test
53-
set_error_handler(function ($errno, $errstr, $errfile, $errline) {
54-
// Don't throw E_WARNING, E_NOTICE, or E_USER_WARNING errors
55-
return true;
56-
});
57-
58-
try {
59-
new FileHandler('/invalid/path/test.log');
60-
} finally {
61-
// Restore the original error handler
62-
restore_error_handler();
82+
$nonWritableDir = sys_get_temp_dir() . '/non_writable_dir_' . uniqid();
83+
mkdir($nonWritableDir);
84+
85+
$mockFileHandler = $this->getMockBuilder(FileHandler::class)
86+
->setConstructorArgs([$nonWritableDir . '/test.log'])
87+
->onlyMethods(['isDirectoryWritable'])
88+
->getMock();
89+
90+
$mockFileHandler->expects($this->once())
91+
->method('isDirectoryWritable')
92+
->willReturn(false);
93+
/** @var AbstractFileHandlerc $mockFileHandler */
94+
$mockFileHandler->__construct($nonWritableDir . '/test.log');
95+
96+
$this->removeDirectory($nonWritableDir);
97+
}
98+
99+
protected function createDirectory($path)
100+
{
101+
if (isset($this->mockFunctions['mkdir'])) {
102+
return call_user_func($this->mockFunctions['mkdir'], $path);
103+
}
104+
105+
return mkdir($path, 0777, true);
106+
}
107+
108+
protected function isDirectoryWritable($path)
109+
{
110+
if (isset($this->mockFunctions['is_writable'])) {
111+
return call_user_func($this->mockFunctions['is_writable'], $path);
63112
}
113+
114+
return is_writable($path);
64115
}
65116

66117
public function testHandleWritesToFile(): void
67118
{
68119
$handler = new FileHandler($this->testLogFile);
69-
$record = $this->createMockRecord('Test message', LogLevel::INFO);
120+
$record = new LogRecord(LogLevel::INFO, 'Test message');
70121

71122
$handler->handle($record);
72123

@@ -78,8 +129,8 @@ public function testHandleWritesToFile(): void
78129
public function testHandleRespectsMinimumLogLevel(): void
79130
{
80131
$handler = new FileHandler($this->testLogFile, LogLevel::WARNING);
81-
$debugRecord = $this->createMockRecord('Debug message', LogLevel::DEBUG);
82-
$warningRecord = $this->createMockRecord('Warning message', LogLevel::WARNING);
132+
$debugRecord = new LogRecord(LogLevel::DEBUG, 'Debug message');
133+
$warningRecord = new LogRecord(LogLevel::WARNING, 'Warning message');
83134

84135
$handler->handle($debugRecord);
85136
$handler->handle($warningRecord);
@@ -99,7 +150,7 @@ public function testHandleUsesFormatter(): void
99150
$handler = new FileHandler($this->testLogFile);
100151
$handler->setFormatter($mockFormatter);
101152

102-
$record = $this->createMockRecord('Test message', LogLevel::INFO);
153+
$record = new LogRecord(LogLevel::INFO, 'Test message');
103154
$handler->handle($record);
104155

105156
$content = file_get_contents($this->testLogFile);
@@ -120,28 +171,10 @@ public function testDestructorClosesFileHandle(): void
120171
$this->assertFalse(is_resource($fileHandleProperty->getValue($handler)));
121172
}
122173

123-
private function createMockRecord(string $message, LogLevel $level): ImmutableValue
174+
public function testLogFileCreatedWithCorrectPermissions(): void
124175
{
125-
return new class($message, $level) implements ImmutableValue {
126-
public function __construct(
127-
public readonly string $message,
128-
public readonly LogLevel $level,
129-
public readonly array $context = [],
130-
public readonly array $extra = [],
131-
public readonly \DateTimeImmutable $datetime = new \DateTimeImmutable()
132-
) {
133-
}
134-
135-
public function toArray(): array
136-
{
137-
return [
138-
'message' => $this->message,
139-
'level' => $this->level,
140-
'context' => $this->context,
141-
'extra' => $this->extra,
142-
'datetime' => $this->datetime,
143-
];
144-
}
145-
};
176+
new FileHandler($this->testLogFile);
177+
$this->assertFileExists($this->testLogFile);
178+
$this->assertEquals('0644', substr(sprintf('%o', fileperms($this->testLogFile)), -4));
146179
}
147180
}

‎tests/Logger/LoggerBuilderTest.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use KaririCode\Contract\Logging\Logger;
88
use KaririCode\Contract\Logging\Structural\HandlerAware;
9+
use KaririCode\Contract\Logging\Structural\ProcessorAware;
910
use KaririCode\Logging\Formatter\LineFormatter;
1011
use KaririCode\Logging\LoggerBuilder;
1112
use PHPUnit\Framework\TestCase;
@@ -26,18 +27,19 @@ public function testWithHandler(): void
2627
$handler = $this->createMock(HandlerAware::class);
2728
$builder = new LoggerBuilder('test');
2829
$builder->withHandler($handler);
29-
30+
/** @var LoggerManager */
3031
$logger = $builder->build();
3132

3233
$this->assertContains($handler, $logger->getHandlers());
3334
}
3435

3536
public function testWithProcessor(): void
3637
{
37-
$processor = $this->createMock(\KaririCode\Contract\Logging\ProcessorAware::class);
38+
$processor = $this->createMock(ProcessorAware::class);
3839
$builder = new LoggerBuilder('test');
3940
$builder->withProcessor($processor);
4041

42+
/** @var LoggerManager */
4143
$logger = $builder->build();
4244

4345
$this->assertContains($processor, $logger->getProcessors());
@@ -49,6 +51,7 @@ public function testWithFormatter(): void
4951
$builder = new LoggerBuilder('test');
5052
$builder->withFormatter($formatter);
5153

54+
/** @var LoggerManager */
5255
$logger = $builder->build();
5356

5457
$this->assertSame($formatter, $logger->getFormatter());

0 commit comments

Comments
 (0)