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.

Sign up for GitHub

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

Jump to bottom

Fix incorrect Annotation::fetchAll() return structure for single Examples attribute #6752

Open
wants to merge 3 commits into
base: 5.1
Choose a base branch
from
Open

Fix incorrect Annotation::fetchAll() return structure for single Examples attribute #6752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8a68a34
Fix incorrect Annotation::fetchAll() return structure for single Exam…
xEdelweiss Apr 12, 2024
ccb7994
Add tests to check Annotation handling of Example and Given
xEdelweiss Apr 24, 2024
dae7d84
Fix PHPCS errors
xEdelweiss May 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Codeception/Util/Annotation.php
View file
Open in desktop
Expand Up @@ -130,6 +130,9 @@ public function fetchAll(string $annotation): array
$attr = $this->attribute($annotation);
if ($attr) {
if (!$attr->isRepeated()) {
if ($annotation === 'example') {
return [$attr->getArguments()];
}
Copy link
Member

Choose a reason for hiding this comment

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

This change broke tests\cli\RunCest.php:runWithExamples

Please fix it.

Copy link
Author

Choose a reason for hiding this comment

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

I've missed this test, but it is the one that highlights the issue and conflicts with the changes I propose in this PR.

It is confusing that a single #[Example] attribute is treated not in the same way as multiple ones.
This also forces you to update attribute arguments when you move from single #[Example] to multiple and vice versa.

Like in this flow:

// write simple test - works

#[Examples([1, 1])]
public function example1(AttrsTester $I, Example $e)
{
    $I->assertEquals($e[1], $e[0]);
}

// duplicate line, update data - fails

#[Examples([1, 1])]
#[Examples([2, 2])]
public function example2(AttrsTester $I, Example $e)
{
    $I->assertEquals($e[1], $e[0]);
}

// unwrap arrays to make it work - works

#[Examples(1, 1)]
#[Examples(2, 2)]
public function example3(AttrsTester $I, Example $e)
{
    $I->assertEquals($e[1], $e[0]);
}

// same in the opposite direction

I see how this change can introduce backward incompatibility for those who have used a single #[Example]. But since the documentation does not describe the specific behavior of a single #[Example], I don't think it's a big issue compared to the improved developer experience.

So I propose to update the tests\cli\RunCest.php:runWithExamples.

I'd appreciate your thoughts on this.

return $attr->getArguments();
}
$attrs = $this->attributes();
Expand Down
77 changes: 77 additions & 0 deletions tests/unit/Codeception/Util/AnnotationTest.php
View file
Open in desktop
Expand Up @@ -92,4 +92,81 @@ public function testSingleLineAnnotation()
->method('testSingleLineAnnotation')
->fetch('value'));
}

public function testFetchAllExamples()
{
$class = new class {
/**
* @example ["example 1/2"]
* @example ["example 2/2"]
*/
public function multipleAnnotations()
{
}

/**
* @example ["example 1/1"]
*/
public function singleAnnotation()
{
}

#[\Codeception\Attribute\Examples('example 1/2')]
#[\Codeception\Attribute\Examples('example 2/2')]
public function multipleAttributes()
{
}

#[\Codeception\Attribute\Examples('example 1/1')]
public function singleAttribute()
{
}
};

$this->assertSame(
['["example 1/2"]', '["example 2/2"]'],
Annotation::forMethod($class, 'multipleAnnotations')->fetchAll('example')
);

$this->assertSame(
['["example 1/1"]'],
Annotation::forMethod($class, 'singleAnnotation')->fetchAll('example')
);

$this->assertSame(
[['example 1/2'], ['example 2/2']],
Annotation::forMethod($class, 'multipleAttributes')->fetchAll('example')
);

$this->assertSame(
[['example 1/1']],
Copy link
Member

Choose a reason for hiding this comment

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

phpcs doesn't like style of this file

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, my bad. Missed PHPCS checks. Fixed.

Annotation::forMethod($class, 'singleAttribute')->fetchAll('example')
);
}

public function testFetchAllGiven()
{
$class = new class {
#[\Codeception\Attribute\Given('given 1/2')]
#[\Codeception\Attribute\Given('given 2/2')]
public function multipleAttributes()
{
}

#[\Codeception\Attribute\Given('given 1/1')]
public function singleAttribute()
{
}
};

$this->assertSame(
['given 1/2', 'given 2/2'],
Annotation::forMethod($class, 'multipleAttributes')->fetchAll('Given')
);

$this->assertSame(
['given 1/1'],
Annotation::forMethod($class, 'singleAttribute')->fetchAll('Given')
);
}
}