Skip to content

Conversation

@matt-allan
Copy link

This PR adds the ability to pass multiple path arguments:

behat features/apples.feature features/pears.feature

We use CircleCI to run a large suite of Behat tests across multiple docker containers. CircleCI can automatically balance tests across the containers to keep the run time as low as possible. The problem is, Circle assumes the test runner will accept multiple file arguments.

I'm working around this at the moment with a bash script and a Behat extension so I can invoke behat like behat --feature apples.feature --feature pears.feature. I can't make an extension that directly changes the paths argument to accept an array because other extensions expect a string.

This is a breaking change, so it will need to wait for Behat 4.0.0.

Backwards (In)compatibility:

  • You can't add an optional argument after an array argument, so any extension adding an optional argument will break.
  • Any extension using $input->getArgument('paths') or $input->setArgument('paths') and expecting a string will break.

What do you think?

@matt-allan matt-allan force-pushed the feature/multiple-paths branch from bf93ca6 to 7e2f746 Compare February 10, 2017 16:00
@gquemener
Copy link

I personnally think this would be an awesome improvement, very usefull on CircleCI indeed to parallelize execution.

* @return string[]
*/
public function getPath()
public function getPaths()
Copy link
Member

Choose a reason for hiding this comment

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

Please also add getPath() with @deprecated tag for BC. It can return the first path given.

parent::__construct($message);

$this->path = $path;
$this->paths = $paths;
Copy link
Member

Choose a reason for hiding this comment

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

For BC please ensure that $paths argument can be either a string or an array. Both must be supported for now.

$this->paths = (array) $paths;

@everzet
Copy link
Member

everzet commented Feb 17, 2017

Any extension using $input->getArgument('paths') or $input->setArgument('paths') and expecting a string will break.

Argh. Good catch :/ There is a way, though. You can switch back to paths being just a singular string and parse it out in the controller. This will force people to use

> behat "pathA pathB"

instead of

> behat pathA pathB

But it is surely better than waiting for 4.0, which is not even scheduled yet.

@stof
Copy link
Member

stof commented Feb 17, 2017

@everzet but would it work for the circleCI features ?

Btw, this would be a nightmare to implement, because you can have spaces inside path, so we would have to provide our own escaping inside this string. And requiring to escape spaces in the path to avoid considering the argument as a path list is a BC break for callers of Behat (and makes it much harder to use Behat by default), which is worse than breaking extensions IMO (especially given that extensions should generally not mess with arguments implemented by another one IMO)

@stof
Copy link
Member

stof commented Feb 17, 2017

thus, if an extension relies on the argument directly, changing the content of the string from a path to a space-delimited list of escaped path would also break BC for the extension, as it would have to change the way it uses the string.

@everzet
Copy link
Member

everzet commented Feb 17, 2017

:/

@gquemener
Copy link

Here's Circle CI behaviour : https://circleci.com/docs/parallel-manual-setup/#using-configuration-files-modifier

By default, the file arguments will be appended to the end of the command.

I'm not sure how it'll react if behat expects it to be quoted, but I wouldn't recommend so.

Well, that's too bad that the argument is already named paths and not path (any historical reason for that?).

What about bumping to 4.0.0 ?

@matt-allan matt-allan force-pushed the feature/multiple-paths branch from 7e2f746 to 574b863 Compare February 17, 2017 14:28
@matt-allan matt-allan force-pushed the feature/multiple-paths branch from 574b863 to 7dc66c8 Compare February 17, 2017 14:41
@matt-allan
Copy link
Author

@everzet I rebased on master and made WrongPathsException backwards compatible.

It sounds like there isn't a good way to prevent breaking BC. I'm personally ok with waiting until 4.0, even if it takes a few years :)

@avant1 avant1 mentioned this pull request Apr 4, 2017
@everzet everzet changed the title [Feature] Accept multiple path arguments Accept multiple path arguments Jun 30, 2017
@pscheit-lillydoo
Copy link

voice-over-sponge-bob-voice:

3 years later

@adrienbrault
Copy link

Hey, is anything else needed for this besides a rebase?

What could I do to help?

@robertfausk
Copy link

@pamil Do you have any plans when this will get merged/released? Cause it is also hindering my git hooks a little bit.

I am using githooks as described in captainhook-git/captainhook#128
to perform only the changed *.feature-files in a pre-commit hook. This currently breaks if there is changed more than one feature file in a commit.

PS: Glad to see you maintaining Behat/Behat.

adrienbrault pushed a commit to golem-ai/Behat that referenced this pull request Jan 4, 2022
@ciaranmcnulty
Copy link
Contributor

Can you please target this to the 4.x branch?

@ciaranmcnulty ciaranmcnulty changed the base branch from master to 4.x June 30, 2022 13:50
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.

8 participants