Skip to content
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

Remove instantiated (child process) objects from TestFailure #1352

Merged
merged 4 commits into from
Jul 27, 2014

Conversation

sun
Copy link
Contributor

@sun sun commented Jul 25, 2014

Resolves #1351

Let's keep communication centralized over there.

* the parent would break the intended encapsulation of process isolation.
*
* @see http://fabien.potencier.org/article/9/php-serialization-stack-traces-and-exceptions
*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To clarify: I'm aware that PHPUnit's current code contains almost no developer documentation at this point. I don't know whether that has been an informed decision or whether it's just caused by laziness.

However, in case of deeply internal PHP gems like this, it's guaranteed that every contributor who studies the code will have no idea why PHPUnit is doing what it's doing (unless she's familiar with the problem space already, which is very unlikely). Having to grep commit logs first, just because some code happens to resolve a complex problem space, presents a barrier to contribution. That's to be avoided.

Therefore, it's best to make it easy for new contributors to understand (at minimum) architecture-level considerations and pitfalls. Doing so is a qualified and verified approach to increase contributions.

(In fact, being deeply involved in one of the largest and most thriving FOSS communities, I won't hesitate to say that this is the key to sustainable success, specifically with regard to the quality and amount of user contributions.)

Copy link
Contributor

Choose a reason for hiding this comment

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

The lack of developer documentation likely stems from the fact that PHPUnit was developed and maintained by a single developer for most of its history. I find notes like this very helpful.

whatthejeff added a commit that referenced this pull request Jul 27, 2014
Remove instantiated (child process) objects from TestFailure
@whatthejeff whatthejeff merged commit b617a1f into sebastianbergmann:master Jul 27, 2014
@whatthejeff
Copy link
Contributor

Very nice addition, @sun! Thanks for all your hard work :)

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.

Process isolation TestResult contains serialized test class upon test failure/exception
2 participants