Simplify Test classes by TavoNiievez · Pull Request #6845 · Codeception/Codeception · GitHub
Skip to content

Simplify Test classes - #6845

Merged
TavoNiievez merged 1 commit into
Codeception:mainfrom
TavoNiievez:simplify_test
Apr 28, 2025
Merged

TavoNiievez merged 1 commit into
Codeception:mainfrom
TavoNiievez:simplify_test

Conversation

@TavoNiievez

Copy link
Copy Markdown
Member

No description provided.

@TavoNiievez
TavoNiievez force-pushed the simplify_test branch 30 times, most recently from 57fd083 to 77c5b6e Compare April 23, 2025 19:33
@TavoNiievez
TavoNiievez force-pushed the simplify_test branch 3 times, most recently from 9cb446e to 739d91e Compare April 23, 2025 23:03
@TavoNiievez
TavoNiievez marked this pull request as ready for review April 23, 2025 23:30
@TavoNiievez

Copy link
Copy Markdown
Member Author

@TavoNiievez
TavoNiievez merged commit 209bc3c into Codeception:main Apr 28, 2025
Comment on lines +84 to +86

@W0rma W0rma May 12, 2025 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@TavoNiievez This check breaks some tests in one of my own projects if I try to access $this->tester. Error message: Undefined property: xxxTest::$tester

The test works fine if I remove the if clause and just call $this->{$property} = $di->instantiate($actor);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@W0rma Sounds like you're right. Could you open a PR with something like this:

$di = $this->getMetadata()->getService('di');

if (($actor = $this->getMetadata()->getCurrent('actor')) &&
    ($property = lcfirst((string) Configuration::config()['actor_suffix']))) {

    $this->{$property} = $di->instantiate($actor);
}

?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@TavoNiievez See #6856

The change fixes the tests of my own project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants