Skip to content

Conversation

saucelabs
Copy link

pass the arg to the constructor up to TestCase, which will keep track of it and hand it down to subprocesses once phpunit-parallel gets released

without this and my recent changes to phpunit, phpunit_selenium can never run with process isolation or parallelism using a public static $browsers array, which is a standard feature of phpunit_selenium

… of it and hand it down to subprocesses once phpunit-parallel gets released
@sebastianbergmann
Copy link
Contributor

PHPUnit_Framework_TestCase::__construct() does not have a fourth argument (and must not get one just because of PHPUnit_Extensions_SeleniumTestCase), so passing the argument along to the parent constructor is pointless.

@saucelabs
Copy link
Author

It's not pointless. Right now if a selenium test case that was constructed with a $browser object tries to run in an extra process, the fourth argument ($browser) gets dropped by TestCase before the subprocess gets started. This could happen with any extension of TestCase that has more constructor arguments.

I also have a patch to TestCase's constructor which grabs the arguments passed into it beyond the third, then passes them down to process-isolated versions of itself through the template

@sebastianbergmann
Copy link
Contributor

Pointless was the wrong word. Your patches should not be necessary, they are working around an issue rather than fixing the root cause. Will have to look at it in more detail.

Joe Mathes added 3 commits October 18, 2011 18:14
@saucelabs
Copy link
Author

Alternative is to make the process-isolation more subclassable in TestCase? The way it uses templates would mean the template itself would need to be subclassable, because afaict the only parts you can modify with prepareTemplate are the {variables}.

Does this block the pull request? If so, it should block the pull request to PHPUnit, because support for it is added there

@saucelabs saucelabs closed this Oct 19, 2011
@saucelabs saucelabs reopened this Oct 19, 2011
@saucelabs
Copy link
Author

whoops! didn't mean to close

@giorgiosironi
Copy link
Owner

I see that you are contributing parallel test execution to PHPUnit,
When PHPUnit 3.7 (or a further release) will be released, will these commits be still valid?

@sebastianbergmann
Copy link
Contributor

I will not merge the SauceLabs patch into PHPUnit 3.7 but rather rewrite PHPUnit's test runner for PHPUnit (Q4 2012 / Q1 2013) from scratch to support parallel and distrbuted test execution using solutions such as ZeroMQ.

@jmathes
Copy link

jmathes commented Feb 6, 2012

Reading this comment is painful. I put a lot of work into the concurrency branch, and I was very excited about using my job at Sauce Labs to contribue to PHPUnit. There were two weeks where I worked 16 hours a day on it.

But, if you don't want to merge it, that means not merging it is best for PHPUnit. So thank you for not merging it. I'll log in as the SauceLabs user and close the pull requests.

@saucelabs saucelabs closed this Feb 6, 2012
@sebastianbergmann
Copy link
Contributor

Your work is not for naught, Joe. You, and SauceLabs, are delivering value to Selenium users now. Although I never got to try the patch myself, I believe that it works just fine for running PHPUnit_Selenium tests in parallel. However, it does not take PHPUnit features such as the @depends into account. For this, and other things, to work properly in the context of parallel test execution the test executor has to be rewritten from scratch. Issue #10 lists other reasons for and goals of this rewrite that I have been putting off way too long.

@jmathes
Copy link

jmathes commented Feb 9, 2012

If the issue is @Depends, I could make it take that into account. I'll go ahead and do that!

I can address the rest of issue #10 too, and add tests for them. The only reason I didn't fix "pass extra args to the constructor up to TestCase, the parent, so it can hand them off to subprocesses" when you mentioned it is because I thought you felt like it wasn't important enough to block the merge, and could be fixed later. I can fix it now.

The entire PHPUnit test suite passes when run in parallel, so any issues that might arise are issues that don't have tests.

In terms of a rewrite, I found the PHPUnit's test runner code to be great. It easy to understand and extend, edit, and improve, because it follows so many good design patterns, like not being tightly coupled. When you need to improve something that's great at being improved, I don't think you should throw it away to improve it.

If you're certain that a rewrite is needed, I'd love to help. I'm willing to do all the work and take notes from you as a director, or any level of involvement you want. We could work together on a branch or a fork.

@DarrenCook
Copy link

Any more activity on this? Being able to run tests in parallel would be great, but I was also wondering how @Depends would work with it. "The entire PHPUnit test suite passes when run in parallel, so any issues that might arise are issues that don't have tests." is a compelling argument. Sebastian, do you have a test that will show some fundamental, unfixable flaw in the proposed patch?
(By the way "Issue #10" has no information, and the link to the original ticket is now a 404 page. What is/was the issue?)

@jmathes
Copy link

jmathes commented May 24, 2012

From the list of issues, I think Sebastian wants the test runner rewritten to support several new features, one of which is parallelism. It sounds like he has a master plan. Sadly it makes him a blocker for parallelism again, but if his estimates are correct, that will only last until Q1 2013.

@Dumk0
Copy link

Dumk0 commented Feb 19, 2013

What is the status of implementing parallelism in phpunit? Waiting for it, and thank You for your project.

@giorgiosironi
Copy link
Owner

Check out https://github.com/brianium/paratest for a drop-in replacement for the phpunit command that wraps it and execute test or test methods in parallel processes. So parallelism is already here.

@jmathes
Copy link

jmathes commented Feb 24, 2013

This branch is never going to be merged. The PHPUnit maintainers want to rewrite the whole test runner and add parallelism then. Their estimate for when it would be done is now in the past. I suggest you make your own wrapper or find one of the many existing ones

@joshua-paddle
Copy link

A year later, and we're still waiting - seems pretty clear that phpunit will never support parallelism. As far as I can see paratest seems to be the only option.

@deepa2083
Copy link

2015 - any updates on this? parallelize phpunit

@jmathes
Copy link

jmathes commented Jul 23, 2015

Nope. This was running, but after a few rebases I still never got it
merged, and eventually Sebastian told me he planned to rewrite the core
runner in a way that would render this useless, so he doesn't plan to ever
merge it. In other words, parallelism is back to being postponed
indefinitely

On Wed, Jul 22, 2015 at 1:43 PM, deepa2083 notifications@github.com wrote:

2015 - any updates on this? parallelize phpunit


Reply to this email directly or view it on GitHub
#64 (comment)
.

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.

8 participants