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

PHP Config - Context #182

Open
wants to merge 1 commit into
base: v3.0
Choose a base branch
from
Open

Conversation

loic425
Copy link
Contributor

@loic425 loic425 commented Dec 9, 2024

@loic425 loic425 marked this pull request as draft December 9, 2024 08:29
@loic425 loic425 force-pushed the php-config/context branch from b3051d3 to 7224c2b Compare December 9, 2024 08:33
@loic425 loic425 marked this pull request as ready for review December 23, 2024 07:35
Copy link
Contributor

@acoulton acoulton left a comment

Choose a reason for hiding this comment

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

Thanks for this @loic425, just got a few changes to suggest (mostly all the same thing).

Joyeux Noël!

@@ -263,62 +312,85 @@ As a matter of fact, Behat gives you ability to do just that. You can
specify arguments required to instantiate your context classes through
same ``contexts`` setting inside your ``behat.yml``:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
same ``contexts`` setting inside your ``behat.yml``:
same ``contexts`` setting inside your ``behat.php``:

Comment on lines +149 to +151
use App\Tests\Behat\Context\FeatureContext;
use App\Tests\Behat\Context\SecondContext;
use App\Tests\Behat\Context\ThirdContext;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should keep the contexts in all these examples in the root namespace, because "out of the box" just following the instructions on this page, these won't be autoloadable by behat (unless you put them in features/bootstrap/App/Tests/Behat/Context/FeatureContext).

Of course I realise that the vast majority of people these days are familiar with how to configure autoloading themselves e.g. through composer, but while we're providing / documenting Behat's custom autoloader I think the docs examples should be consistent with how that behaves.

Suggested change
use App\Tests\Behat\Context\FeatureContext;
use App\Tests\Behat\Context\SecondContext;
use App\Tests\Behat\Context\ThirdContext;
use FeatureContext;
use SecondContext;
use ThirdContext;

<?php
// behat.php

use App\Tests\Behat\Context\MyAwesomeContext;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
use App\Tests\Behat\Context\MyAwesomeContext;
use MyAwesomeContext;

Comment on lines +241 to +242
use App\Tests\Behat\Context\MyAwesomeContext;
use App\Tests\Behat\Context\MyWickedContext;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
use App\Tests\Behat\Context\MyAwesomeContext;
use App\Tests\Behat\Context\MyWickedContext;
use MyAwesomeContext;
use MyWickedContext;

<?php
// behat.php

use App\Tests\Behat\Context\MyAwesomeContext;
Copy link
Contributor

Choose a reason for hiding this comment

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

Especially in this example, as the example context class itself is not namespaced and says it is in features/bootstrap/MyAwesomeContext.php:

Suggested change
use App\Tests\Behat\Context\MyAwesomeContext;
use MyAwesomeContext;


.. code-block:: yaml
use App\Tests\Behat\Context\MyAwesomeContext;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
use App\Tests\Behat\Context\MyAwesomeContext;
use MyAwesomeContext;

<?php
// behat.php

use App\Tests\Behat\Context\MyAwesomeContext;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
use App\Tests\Behat\Context\MyAwesomeContext;
use MyAwesomeContext;

<?php
// behat.php

use App\Tests\Behat\Context\MyAwesomeContext;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
use App\Tests\Behat\Context\MyAwesomeContext;
use MyAwesomeContext;

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