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

Fix for #88 #129

Merged
merged 1 commit into from
Dec 24, 2021
Merged

Fix for #88 #129

merged 1 commit into from
Dec 24, 2021

Conversation

DonCallisto
Copy link
Contributor

@DonCallisto
Copy link
Contributor Author

@pamil forgot to add bind section to first test. This way it was failing due to misconfiguration (no service for AbstractBrowser). Just let me try to fix and see if still fails

@DonCallisto DonCallisto force-pushed the issue_#88 branch 8 times, most recently from 58b9f67 to 7e13510 Compare June 18, 2020 17:41
@DonCallisto
Copy link
Contributor Author

@pamil umh, I'm a little bit confused. That's the situation where it should not pass. Am I missing something?

Copy link
Contributor

@cv65kr cv65kr left a comment

Choose a reason for hiding this comment

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

Hey

To generate failed test you must provide some changes.

I changed temporary in tests/Behat/Context/TestContext.php line 120
from

'test' => $this->getEnvironment() === 'test',

to

'test' => false

Because this problem exists when enviroment is set to another value than test. Thats should be do in separate step or just provide as parameter in step.

image

@@ -131,3 +131,48 @@ Feature: Autowiring contexts
"""
When I run Behat
Then it should pass

Scenario: Autowiring a context with test client
Given a feature file containing:
Copy link
Contributor

Choose a reason for hiding this comment

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

Given a working Symfony application with SymfonyExtension configured
And a feature file containing

Because you must use CompilerPass from src/Bundle/DependencyInjection/FriendsOfBehatSymfonyExtensionExtension:process.

@DonCallisto
Copy link
Contributor Author

@cv65kr why environment should be different than test?
This extension is used specifically in test environment, so why should the test case take into account a different env?

@cv65kr
Copy link
Contributor

cv65kr commented Jun 19, 2020

client.test is always available when environment is set to test (https://symfony.com/doc/current/reference/configuration/framework.html#test).

In previous version of this extension you have a possibility to run Behat tests with different environment then test.

@DonCallisto
Copy link
Contributor Author

Umh, but when I run tests in a Symfony app (and so #88 issue arise), I suppose that environment value was/is test.
Am I missing something?

@cv65kr
Copy link
Contributor

cv65kr commented Jun 19, 2020

Little weird, first of all try to set up test to true in your configuration then we will look if problem still exist.

@DonCallisto
Copy link
Contributor Author

@cv65kr check out now. env is setted to test and test is green.

@cv65kr
Copy link
Contributor

cv65kr commented Jun 19, 2020

I mean

framework:
    test: true

in config/framework.yml because that's enable test.client service (https://github.com/symfony/symfony/blob/3e57f1f49f993b275fc014e029fc542ead2c15fb/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php#L261)

Let's try. If you change this value to false then test should failed. Maybe when APP_ENV is set to test this configuration is set up automatically.

Also try debug FrameworkExtension to make sure if test.client is not removed.

@DonCallisto
Copy link
Contributor Author

@cv65kr this should be the same. Am I wrong?
If I change the value to dev, the test fails.

@cv65kr
Copy link
Contributor

cv65kr commented Jun 19, 2020

I lost myself 😄 same with?

@DonCallisto
Copy link
Contributor Author

What I've done is (practically) the same of

framework:
    test: true

Isn't it?

@cv65kr
Copy link
Contributor

cv65kr commented Jun 19, 2020

Right, that should be equals.

@Yozhef
Copy link
Contributor

Yozhef commented Dec 14, 2021

@DonCallisto can you please update your branch in master - we need run GitHub Actions

@DonCallisto
Copy link
Contributor Author

@Yozhef done

@Yozhef
Copy link
Contributor

Yozhef commented Dec 14, 2021

@cv65kr tell me do you have any other comments?

@Yozhef Yozhef merged commit de01d58 into FriendsOfBehat:master Dec 24, 2021
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.

4 participants