Skip to content

'More than two positional arguments provided.' breaks backward compatibility #655

Closed
tmysik opened this Issue Sep 19, 2012 · 14 comments

6 participants

@tmysik
tmysik commented Sep 19, 2012

Right now, we run our tests via our own TestSuite [1]:

/usr/bin/phpunit  NetBeansSuite /opt/netbeans-dev/php/phpunit/NetBeansSuite.php run=/home/gapon/NetBeansProjects/Calculator/test

This worked fine until now. PHPUnit 3.7.0 says:

More than two positional arguments provided.

Is this expected behaviour? Is running custom test suites via CLI supported scenario? Any help would be really appreciated.

Thanks.
[1] Reasons why we use our own test suite can be found here: https://netbeans.org/bugzilla/show_bug.cgi?id=167519

@sebastianbergmann

That check was introduced in 6ac554e and I do not recall right now why it was added. I was not aware of any issues related to this until now.

@tmysik
tmysik commented Sep 19, 2012

Thanks for your reply. I just found out that easy hotfix on NetBeans' side is to remove NetBeansSuite:

/usr/bin/phpunit /opt/netbeans-dev/php/phpunit/NetBeansSuite.php run=/home/gapon/NetBeansProjects/Calculator/test

It seems to work, for PHPUnit 3.7.0 as well as for 3.6.x. Just wanted to be sure that we don't use any unsupported/deprecated feature. Or what is the correct way to run TestSuite via CLI - is there such possibility officially?

BTW feel free to close this issue as INVALID if you think that it should not be supported.

Thanks a lot.

@edorian
Collaborator
edorian commented Sep 19, 2012

This was added as a saveguard against people providing CLI arguments that PHPUnits parser silently ignored

E.g. phpunit foo.php fooTest --coverage-text never produced any coverage and that was quite unintuitive and hard to debug.

I'm not sure what run=/path does exactly to be honest. Is this something you use / parse in your TestSuite generator?

Running different test suites from the CLI can be done using the path, different xml files or using --filter. But providing a custom parsed CLI parameter is something that the update broke.

@tmysik
tmysik commented Sep 19, 2012

Thanks for your answer.

I'm not sure what run=/path does exactly to be honest. Is this something you use / parse in your TestSuite generator?

Yes (NetBeans needs to tell the test suite which file/folder should be run).

As I wrote before, since we have now the work around, I am OK with it. But not sure what we could do if we had more than one parameter for the test suite (I still think that it should be supported - why not?).

Thanks guys.

@brokenthumbs

Wanted to chime in. I also ran into the same issue. We use multiple custom parsed CLI parameters with PHPUnit to specify which server and port to run our Selenium tests on.

phpunit foo.php --host windows --port 4441

We've found a work around to circumvent the safeguard.

phpunit foo.php "--host windows --port 4441"

Taking the two parameters as a string and then parsing the parameters as normal.

I do see why it makes sense to add the safeguard, but I would like the option to pass multiple custom parameters without passing it as a single string.

@tmysik
tmysik commented Sep 21, 2012

I do see why it makes sense to add the safeguard, but I would like the option to pass multiple custom parameters without passing it as a single string.

Seems to me to be a good idea.

@edorian
Collaborator
edorian commented Sep 21, 2012

The fact that PHPUnit accepts that now is just coincidental. Tbh. I wasn't aware that many folks run tests like "phpunit class.php" anyways as most of what I'm seeing is people using the phpunit.xml and filters.

Apart from that the change seems to cause more issues than it's worth and if @sebastianbergmann agrees I'd just remove it again.

Passing custom paramers to phpunit via cli args might be a ugly hack but using _ENV isn't all that much nicer. Until we have something build in that can be a nice replacement i guess it's ok to not break that :)

@tmysik
tmysik commented Sep 21, 2012

Just one minor note:

people using the phpunit.xml and filters

Unfortunately, this does not work for NetBeans since users can have their own XML configuration (so the only way for NetBeans to pass parameters to PHPUnit is via CLI).

@edorian
Collaborator
edorian commented Sep 21, 2012

Yes. I definitely see the point for IDE vendors @tmysik :) phpstorm solved that with a custom runner script that does lots of crazy things and netbeans also found a nice solutions. I was just describing what i had in mind when accepting the PR for this feature

Hence I'm in favor of not getting in your way since passing _ENV variables is presumably more of an issue for you (due to cross plattform issues).

@tmysik
tmysik commented Sep 22, 2012

Thanks a lot. However, as I already wrote, I am fine with the current state since we found a way how to solve our problem ;)

@edorian edorian added a commit that closed this issue Sep 24, 2012
@edorian edorian Closes #655. BC issue in 3.7 fixed by: Revert "Positional argument co…
…unt check introduced"

This reverts commit 6ac554e.

Conflicts:

	PHPUnit/TextUI/Command.php
cd92dd4
@edorian edorian closed this in cd92dd4 Sep 24, 2012
@edorian edorian added a commit that referenced this issue Sep 24, 2012
@edorian edorian Changelog for #655 54e0ae0
@rukbat
rukbat commented Oct 3, 2012

Hi, it is possible to upgrade to PHPUnit 3.7.2 via PEAR?

~$ sudo pear remote-list -c phpunit

Channel phpunit Available packages:

Package Version
...
PHPUnit 3.7.1

@sebastianbergmann
Owner

PHPUnit 3.7.2 has not been released yet.

@neufena
neufena commented Oct 3, 2012

Any idea when 3.7.2 will be release or if there's a way to patch this fix manually without pear?

@edorian
Collaborator
edorian commented Oct 3, 2012

@neufena I can't say anything definitively but i expect 3.7.2 to be released in the next couple of weeks.

As a patch you can use the comment linked here to revert the change ( cd92dd4 )

@greglamb greglamb pushed a commit to greglamb/phpunit that referenced this issue Apr 19, 2013
@edorian edorian Closes #655. BC issue in 3.7 fixed by: Revert "Positional argument co…
…unt check introduced"

This reverts commit 6ac554e.

Conflicts:

	PHPUnit/TextUI/Command.php
193bf77
@greglamb greglamb pushed a commit to greglamb/phpunit that referenced this issue Apr 19, 2013
@edorian edorian Changelog for #655 45b4296
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.