Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#145: Fix bug with empty arguments #170

Merged
merged 9 commits into from
Jan 10, 2025
Merged

Conversation

yogyrton
Copy link
Contributor

Comment on lines 137 to 144
if ($isClass) {
$reflectionMethod = new \ReflectionMethod($class, $function);
$parameters = $reflectionMethod->getParameters();
$requiredParametersCount = count(array_filter($parameters, fn($param) => !$param->isOptional()));
} else {
$parameters = $actual;
$requiredParametersCount = count(array_filter($actual, fn ($item) => $item !== self::OPTIONAL_ARGUMENT_NAME));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's move it to the separate method which will return [$parameters, $requiredParametersCount]

@@ -134,21 +134,43 @@ protected function assertArguments(
$expectedCount = count($expected);
$actualCount = count($actual);

if ($expectedCount !== $actualCount) {
if ($isClass) {
$reflectionMethod = new \ReflectionMethod($class, $function);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$reflectionMethod = new \ReflectionMethod($class, $function);
$reflectionMethod = new ReflectionMethod($class, $function);

if ($isClass) {
$reflectionMethod = new \ReflectionMethod($class, $function);
$parameters = $reflectionMethod->getParameters();
$requiredParametersCount = count(array_filter($parameters, fn($param) => !$param->isOptional()));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$requiredParametersCount = count(array_filter($parameters, fn($param) => !$param->isOptional()));
$requiredParametersCount = count(array_filter($parameters, fn ($param) => !$param->isOptional()));


$message = ($isClass)
? "Class '{$class}'\nMethod: '{$function}'\nMethod call index: {$callIndex}"
: "Namespace '{$class}'\nFunction: '{$function}'\nCall index: {$callIndex}";

foreach ($actual as $index => $argument) {
$expectedArgument = $expected[$index] ?? null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

as discussed, please check that this is realy need

@DenTray DenTray assigned yogyrton and unassigned DenTray Dec 23, 2024
@yogyrton yogyrton assigned DenTray and unassigned yogyrton Dec 31, 2024
): void {
$reflection = $isClass
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$reflection = $isClass
$reflection = ($isClass)


$this->fillOptionalArguments($parameters, $actual, $expected, $isClass);

$message = $isClass
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$message = $isClass
$message = ($isClass)


protected function assertArgumentCount(int $expectedCount, int $actualCount, int $requiredParametersCount, string $function): void
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
protected function assertArgumentCount(int $expectedCount, int $actualCount, int $requiredParametersCount, string $function): void
protected function assertArgumentsCount(int $expectedCount, int $actualCount, int $requiredParametersCount, string $function): void

$expectedCount = count($expected);
$actualCount = count($actual);
$parameters = $reflection->getParameters();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$parameters = $reflection->getParameters();
$reflectionArgs = $reflection->getParameters();

Comment on lines 142 to 144
$requiredParametersCount = count(array_filter($parameters, fn ($param) => !$param->isOptional()));

$this->assertArgumentCount($expectedCount, $actualCount, $requiredParametersCount, $function);
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's move counting into the assertArgumentCount function

Suggested change
$requiredParametersCount = count(array_filter($parameters, fn ($param) => !$param->isOptional()));
$this->assertArgumentCount($expectedCount, $actualCount, $requiredParametersCount, $function);
$this->assertArgumentCount($expected, $actual, $reflectionArgs, $function);

Comment on lines 106 to 108
$this->assertEquals('mockFunction', ($mock->mockFunction('firstRequired', 'secondRequired')));
$this->assertEquals('mockFunction', ($mock->mockFunction('firstRequired', 'secondRequired', 'firstOptional')));
$this->assertEquals('mockFunction', ($mock->mockFunction('firstRequired', 'secondRequired', 'firstOptional', 'secondOptional')));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$this->assertEquals('mockFunction', ($mock->mockFunction('firstRequired', 'secondRequired')));
$this->assertEquals('mockFunction', ($mock->mockFunction('firstRequired', 'secondRequired', 'firstOptional')));
$this->assertEquals('mockFunction', ($mock->mockFunction('firstRequired', 'secondRequired', 'firstOptional', 'secondOptional')));
$this->assertEquals('mockFunction', $mock->mockFunction('firstRequired', 'secondRequired'));
$this->assertEquals('mockFunction', $mock->mockFunction('firstRequired', 'secondRequired', 'firstOptional'));
$this->assertEquals('mockFunction', $mock->mockFunction('firstRequired', 'secondRequired', 'firstOptional', 'secondOptional'));

Comment on lines 114 to 117
$this->functionCall('mockFunction', ['firstRequired', 'secondRequired'], 'mockFunction'),
]);

$this->assertNotEquals('result', $mock->mockFunction('firstRequired', 'secondRequired'));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$this->functionCall('mockFunction', ['firstRequired', 'secondRequired'], 'mockFunction'),
]);
$this->assertNotEquals('result', $mock->mockFunction('firstRequired', 'secondRequired'));
$this->functionCall('mockFunction', ['firstRequired', 'secondRequired'], 'mocked_result'),
]);
$this->assertEquals('mocked_result', $mock->mockFunction('firstRequired', 'secondRequired'));

$this->assertEquals('mockFunction', ($mock->mockFunction('firstRequired', 'secondRequired', 'firstOptional', 'secondOptional')));
}

public function testMockFunctionInClassWhenDifferentResult()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public function testMockFunctionInClassWhenDifferentResult()
public function testMockClassMethodCheckMockedResult()

$this->assertNotEquals('result', $mock->mockFunction('firstRequired', 'secondRequired'));
}

public function testMockFunctionInClassWhenLessRequiredParameters()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public function testMockFunctionInClassWhenLessRequiredParameters()
public function testMockClassMethodWhenLessRequiredParameters()

}

public function testAssertArgumentMismatchBetweenExpectedAndActualArguments()
public function testMockFunctionInClassWhenMoreExpectedParameters()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public function testMockFunctionInClassWhenMoreExpectedParameters()
public function testMockClassMethodWhenMoreExpectedParameters()

@DenTray DenTray assigned yogyrton and unassigned DenTray Jan 9, 2025
@yogyrton yogyrton assigned DenTray and unassigned yogyrton Jan 9, 2025
$requiredParametersCount = count(array_filter($actual, fn ($item) => $item !== self::OPTIONAL_ARGUMENT_NAME));
$this->assertFalse(
$expectedCount < $requiredParametersCount,
"Failed assert that function {$function} was called with {$expectedCount} require arguments, actually it calls with {$requiredParametersCount} require arguments."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Failed assert that function {$function} was called with {$expectedCount} require arguments, actually it calls with {$requiredParametersCount} require arguments."
"Failed assert that function {$function} was called with {$expectedCount} arguments, actually it has {$requiredParametersCount} required arguments."

? "Class '{$class}'\nMethod: '{$function}'\nMethod call index: {$callIndex}"
: "Namespace '{$class}'\nFunction: '{$function}'\nCall index: {$callIndex}";
if (!$isClass && $actual[$index] === 'optionalParameter') {
$actual[$index] = $expected[$index] ?? $parameter->getDefaultValue();
Copy link
Collaborator

Choose a reason for hiding this comment

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

please move actual args filling before the expected args filling

Suggested change
$actual[$index] = $expected[$index] ?? $parameter->getDefaultValue();
$actual[$index] = $parameter->getDefaultValue();

@DenTray DenTray assigned yogyrton and unassigned DenTray Jan 10, 2025
@yogyrton yogyrton assigned DenTray and unassigned yogyrton Jan 10, 2025
@DenTray DenTray merged commit 2f40794 into master Jan 10, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants