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

-- separates options from arguments in codecept run #3615

Merged
merged 1 commit into from Oct 22, 2016

Conversation

Projects
None yet
2 participants
@Naktibalda
Member

Naktibalda commented Oct 17, 2016

Fixes #3614

This is a correct way to handle --
But it is potentially breaking, because commands like codecept run --coverage-html -- unit --steps will no longer work.
The correct way to write that one is codecept run --coverage-html --steps -- unit

-- separates options from arguments in codecept run
Fixes #3614

This is a correct way to handle `--`
But it is potentially breaking, because commands like `codecept run --coverage-html -- unit --steps` will no longer work.
The correct way to write that one is `codecept run --coverage-html --steps -- unit`
@Naktibalda

This comment has been minimized.

Show comment
Hide comment
@Naktibalda

Naktibalda Oct 19, 2016

Member

@DavertMik any comments?

Member

Naktibalda commented Oct 19, 2016

@DavertMik any comments?

@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Oct 19, 2016

Member

I think I will add some by the end of the week. I didn't have chance to look into it deeply

Member

DavertMik commented Oct 19, 2016

I think I will add some by the end of the week. I didn't have chance to look into it deeply

@Naktibalda

This comment has been minimized.

Show comment
Hide comment
@Naktibalda

Naktibalda Oct 22, 2016

Member

Actually there is no bc-break, any attempt to use -- resulted in InvalidArgumentException before this patch.

Member

Naktibalda commented Oct 22, 2016

Actually there is no bc-break, any attempt to use -- resulted in InvalidArgumentException before this patch.

@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Oct 22, 2016

Member

Ok, so I have almost no arguments to not merge it.

Except that -- means passing variables directly to some underlying runner.

When we do

composer exec codecept run -- --group t

we are telling composer not to parse --group but bypass it to codecept runner. In case of Codeception we are not bypassing variables to no other app. We actually could pass them to PHPUnit, but we don't, we use only our runner and collect all variables here.

So maybe it's not correct to use -- like this. However, I don't see any other option, and I'm not a Linux expert so I can be mistaken in this case

Member

DavertMik commented Oct 22, 2016

Ok, so I have almost no arguments to not merge it.

Except that -- means passing variables directly to some underlying runner.

When we do

composer exec codecept run -- --group t

we are telling composer not to parse --group but bypass it to codecept runner. In case of Codeception we are not bypassing variables to no other app. We actually could pass them to PHPUnit, but we don't, we use only our runner and collect all variables here.

So maybe it's not correct to use -- like this. However, I don't see any other option, and I'm not a Linux expert so I can be mistaken in this case

@Naktibalda

This comment has been minimized.

Show comment
Hide comment
@Naktibalda

Naktibalda Oct 22, 2016

Member

We are using Symfony Console component, so it is best to adhere to its standards: http://symfony.com/doc/current/components/console/console_arguments.html

Also the Usage section of codecept run looks like this:

Usage:
  run [options] [--] [<suite>] [<test>]
Member

Naktibalda commented Oct 22, 2016

We are using Symfony Console component, so it is best to adhere to its standards: http://symfony.com/doc/current/components/console/console_arguments.html

Also the Usage section of codecept run looks like this:

Usage:
  run [options] [--] [<suite>] [<test>]
@DavertMik

This comment has been minimized.

Show comment
Hide comment
@DavertMik

DavertMik Oct 22, 2016

Member

Thanks for pointing. Yes, you are absolutely right, here is the quote from http://docopt.org

A double dash "--", when not part of an option, is often used as a convention to separate options and positional arguments, in order to handle cases when, e.g., file names could be mistaken for options. In order to support this convention, add "[--]" into your patterns before positional arguments.

Usage: my_program [options] [--] ...
Apart from this special meaning, "--" is just a normal command, so you can apply any previously-described operations, for example, make it required (by dropping brackets

So I'm merging. Thank you!

Member

DavertMik commented Oct 22, 2016

Thanks for pointing. Yes, you are absolutely right, here is the quote from http://docopt.org

A double dash "--", when not part of an option, is often used as a convention to separate options and positional arguments, in order to handle cases when, e.g., file names could be mistaken for options. In order to support this convention, add "[--]" into your patterns before positional arguments.

Usage: my_program [options] [--] ...
Apart from this special meaning, "--" is just a normal command, so you can apply any previously-described operations, for example, make it required (by dropping brackets

So I'm merging. Thank you!

@DavertMik DavertMik merged commit 98372b2 into 2.2 Oct 22, 2016

5 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
semaphoreci The build passed on Semaphore.
Details

@DavertMik DavertMik deleted the Naktibalda-codecept-run branch Oct 22, 2016

chris1312 added a commit to chris1312/Codeception that referenced this pull request Jun 16, 2017

-- separates options from arguments in codecept run (#3615)
Fixes #3614

This is a correct way to handle `--`
But it is potentially breaking, because commands like `codecept run --coverage-html -- unit --steps` will no longer work.
The correct way to write that one is `codecept run --coverage-html --steps -- unit`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment