Skip to content

Conversation

jwage
Copy link

@jwage jwage commented Apr 12, 2019

At my job we had 1 hour of functional tests. We built https://github.com/jwage/phpchunkit on top of phpunit to get the chunking functionality and with this we were able to achieve test builds in ~3 minutes. I thought it might be useful to have this built in to phpunit. If you use travis, circle ci, etc. it becomes trivial to configure multiple jobs, one for each chunk.

Before I continue work on this I wanted to know if something like this would be accepted? I am new to the internals of phpunit so I am not 100% sure if I even implemented this functionality the "correct" way.

Example, run your tests in two separate chunks across two different servers/processes

server1

$ phpunit --chunk=1 --num-chunks=2 --group=functional

server2

$ phpunit --chunk=2 --num-chunks=2 --group=functional

Thoughts?

$this->chunk = $args['chunk'];
$this->numChunks = $args['numChunks'];
$this->numTests = count($iterator);
$this->numTestsPerChunk = floor($this->numTests / $this->numChunks);
Copy link
Author

Choose a reason for hiding this comment

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

Maybe we can use array_chunk here to simplify the logic. Let php chunk it and then in accept() we can check if the current test is in the chunk being ran.

@sebastianbergmann
Copy link
Owner

My initial thought is that I think that I prefer the approach outlined in #3387. That way, PHPUnit is used to create a list of tests and can be given a list of tests to execute. In between those two steps, that list of tests can be chunked, filtered, sorted, etc. outside of PHPUnit.

@jwage
Copy link
Author

jwage commented Apr 13, 2019

Interesting. Thanks for the link.

I did something similar here to achieve my parallelization https://github.com/jwage/phpchunkit

I just built my own runner that finds the tests, chunks them and generates a temporary phpunit xml file to run with.

While what you describe makes sense, it leaves the responsibility up to the end user to implement something custom. Which is not necessarily bad, it is just a higher barrier. With some kind of built in chunking plus travis, circleci, etc. it is just a matter of configuration to achieve parallel test running. Is it possible to make it that easy?

@jwage
Copy link
Author

jwage commented Apr 14, 2019

Shall we close this then?

@jwage jwage closed this Apr 14, 2019
@codecov
Copy link

codecov bot commented Apr 14, 2019

Codecov Report

Merging #3605 into master will decrease coverage by 0.04%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3605      +/-   ##
============================================
- Coverage     82.85%   82.81%   -0.05%     
- Complexity     3713     3721       +8     
============================================
  Files           144      145       +1     
  Lines          9647     9671      +24     
============================================
+ Hits           7993     8009      +16     
- Misses         1654     1662       +8
Impacted Files Coverage Δ Complexity Δ
src/Framework/TestSuiteIterator.php 100% <100%> (ø) 10 <1> (+1) ⬆️
src/Runner/Filter/ChunkIterator.php 100% <100%> (ø) 3 <3> (?)
src/TextUI/TestRunner.php 68.46% <20%> (-0.38%) 297 <0> (+2)
src/TextUI/Command.php 70.35% <33.33%> (-0.39%) 209 <0> (+2)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 893a342...8480d3d. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 14, 2019

Codecov Report

Merging #3605 into master will decrease coverage by 0.04%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3605      +/-   ##
============================================
- Coverage     82.85%   82.81%   -0.05%     
- Complexity     3713     3721       +8     
============================================
  Files           144      145       +1     
  Lines          9647     9671      +24     
============================================
+ Hits           7993     8009      +16     
- Misses         1654     1662       +8
Impacted Files Coverage Δ Complexity Δ
src/Framework/TestSuiteIterator.php 100% <100%> (ø) 10 <1> (+1) ⬆️
src/Runner/Filter/ChunkIterator.php 100% <100%> (ø) 3 <3> (?)
src/TextUI/TestRunner.php 68.46% <20%> (-0.38%) 297 <0> (+2)
src/TextUI/Command.php 70.35% <33.33%> (-0.39%) 209 <0> (+2)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 893a342...2a6d96a. Read the comment docs.

@sebastianbergmann
Copy link
Owner

I need to think about this some more. But it's the weekend, you know ;-) Maybe @epdenouden has some input?

@theofidry
Copy link
Contributor

Random user opinion: I quite miss parallelism in PHP and I think it's hurting a lot the PHP ecosystem especially for testing frameworks. There is quite a few attempts in the wild but they are all unreliable because:

  • Requires to re-implement the test runner (e.g. for paratest for PHPUnit) which is bound to be incomplete or getting out of sync one way or another
  • Requires a dubious PHP extension (I have high hopes on krakjoe/parallel but it's not widely used yet and more importantly not in the core and likely requires one to recompile his/her PHP binary since requires the ZTS distribution)

Anyway just to say from my PoV both approach suggested here and in #3387 are perfectly valid... I quite like #3387 since it opens the door to more advanced stuff. I however dislike that it's pushing the parallelism integration down to third-party instead of being handled by PHPUnit as this will most likely result in a small adoption rate.

The solution suggested here however is elegant and pragmatic: it won't cover all cases but will get the job done for most people. So maybe a solution would be to:

This way you provide a minimalist yet good way to parallelise PHPUnit tests in the core and for the few for whom it's not enough, they have an extension point to work on instead of trying to push more things in PHPUnit guarding it from becoming overly complex and bloated for this extra feature.

@jwage
Copy link
Author

jwage commented Jun 11, 2019

In my opinion, even if PHP has good parallelism support, really big test suites will still need to be handled differently because you need to be able to run the chunks across a BIG infrastructure, not just one server. For example, with this PR it would be easily possible to have a process with docker that spins up all the servers need to run all the chunks in parallel across many servers, and tear it all down when done. This PR enables people to generate the chunks and run them directly with PHPUnit, then they can decide to either parallelize those chunks in one server and multiple PHP threads, or across many servers however they are able to set it up.

I am starting a new project right now where we are modernizing a legacy application and we have unit, functional and selenium tests. The selenium tests will quickly be very slow when ran serially and I have the infrastructure to be able to run those Selenium tests in parallel, I just need the mechanism to do it. This is why I think PHPUnit should have this mechanism built in.

Also, I've seen this type of pattern appear multiple times over the years. Some people manually do the chunking by using groups (I think Etsy was the first I saw to do this) and some create different test suites...then they run the different testsuites/groups across multiple jobs in Jenkins for example.

@sebastianbergmann
Copy link
Owner

Thank you for your contribution. I appreciate the time you invested in preparing this pull request. However, I have decided not to merge it.

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