Skip to content

Removed superfluous service validation logic #44

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

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Removed superfluous service validation logic #44

merged 1 commit into from
Nov 20, 2020

Conversation

TavoNiievez
Copy link
Member

TL;DR: The validation that the service exists was already done when obtaining the service. The code that is removed in this PR was not necessary.

Click to see full description.
$this->grabService() validates that the service exists and otherwise fails the test showing a very descriptive message.

public function grabService(string $service)
{
$container = $this->_getContainer();
if (!$container->has($service)) {
$this->fail("Service $service is not available in container.
If the service isn't injected anywhere in your app, you need to set it to `public` in your `config/services_test.php`/`.yaml`,
see https://symfony.com/doc/current/testing.html#accessing-the-container");
}
return $container->get($service);
}

I had ignored that this implicit validation already existed when writing the functions that i edit in this PR:

logout, seeInSession, amOnAction, seeAuthentication, seeUserHasRole, dontSeeAuthentication, grabParameter, seeCurrentActionIs, seeUserPasswordDoesNotNeedRehash, amLoggedInAs.

@TavoNiievez TavoNiievez marked this pull request as ready for review November 19, 2020 17:46
@TavoNiievez TavoNiievez merged commit 2ea7ed1 into Codeception:master Nov 20, 2020
@TavoNiievez TavoNiievez deleted the service_fixes branch November 20, 2020 15:46
@TavoNiievez TavoNiievez added this to the 1.4.0 milestone Nov 21, 2020
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.

1 participant