Skip to content

Conversation

@loic425
Copy link
Contributor

@loic425 loic425 commented Dec 4, 2024

I'm wondering if we should implement withFilters(FilterInterface ...$filters) instead.
But for now, we have implemented them like this on Profile.

use Behat\Config\Config;
use Behat\Config\Profile;
use Behat\Config\Suite;
use Behat\Config\Filter\RoleFilter;

$profile = (new Profile('default'))
    ->withSuite(
        (new Suite('little_kid'))
            ->withContexts(LittleKidContext::class)
            ->withFilter(new RoleFilter('little kid'))
        )
    ->withSuite(
        (new Suite('big_brother'))
            ->withContexts(BigBrotherContext::class)
            ->withFilter(new RoleFilter('big brother'))
        )
;

return (new Config())->withProfile($profile);

@loic425
Copy link
Contributor Author

loic425 commented Dec 4, 2024

@acoulton @carlos-granados
easy to review now we have the previous one.

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.

LGTM.

I think I'd stick with ->withFilter() - there will often only be one, or at most a very short list, and in time we might want to add helpers like e.g. (new TagFilter())->include('ui')->exclude('php84') which would be neatest if each filter was in a separate method call.

@carlos-granados
Copy link
Contributor

LGTM.

I think I'd stick with ->withFilter() - there will often only be one, or at most a very short list, and in time we might want to add helpers like e.g. (new TagFilter())->include('ui')->exclude('php84') which would be neatest if each filter was in a separate method call.

Agreed

Copy link
Contributor

@carlos-granados carlos-granados left a comment

Choose a reason for hiding this comment

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

Looks good, just one comment

], $config->toArray());
}

public function testAddingFilters(): void
Copy link
Contributor

Choose a reason for hiding this comment

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

You are missing the test to confirm that you cannot repeat a filter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@loic425 loic425 force-pushed the php-config-suite-filters branch from d3fc8ef to 487d815 Compare December 4, 2024 14:12
Copy link
Contributor

@carlos-granados carlos-granados left a comment

Choose a reason for hiding this comment

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

Thanks @loic425

@carlos-granados carlos-granados merged commit a34bec2 into Behat:master Dec 4, 2024
17 checks passed
@loic425 loic425 deleted the php-config-suite-filters branch December 4, 2024 14:52
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.

3 participants