Replace symfony/yaml #860

Closed
whatthejeff opened this Issue Mar 13, 2013 · 7 comments

4 participants

@whatthejeff
Collaborator

Requiring symfony/yaml causes dependency issues for people testing symfony projects:

Additionally, symfony/yaml has been the source of a number of issues as of late:

The symfony/yaml component is only used for TAP logging in PHPUnit–which is actually overkill as TAP logging only requires the ability to produce a tiny subset of YAML. I propose we replace the symfony/yaml component with a standalone YAMLish producer.

@whatthejeff
Collaborator

Another way to solve the dependency issues is to relax the version requirements and do some sort of version juggling inside PHPUnit. I'd rather just replace the component, though.

@sebastianbergmann

A while ago @edorian had the idea to "bundle" Symfony YAML and put its classes into a different namespace thus isolating it from the "real" Symfony YAML.

Also note that DbUnit uses Symfony YAML as well.

@flack

Using a different namespace (preferably something clearly belonging to PHPUnit) sounds like something that might work. Personally, I think I'd rather replace the component with some custom code, but that's not my call.

I mean, code reuse is always good, but in this case, it interferes with the application's primary task: If the YAML version used by my application during the tests is different from the one my application uses in production (because my app depends on a different version, which is never used during the tests, because phpunit already loaded its own version), then the usefulness of unit testing is somewhat reduced, because I'm not really testing the same code that I actually run.

@whatthejeff whatthejeff added a commit to whatthejeff/phpunit that referenced this issue Mar 14, 2013
@whatthejeff whatthejeff Fixes #860
 * Relax version requirements for `symfony/yaml`.
a7c1da3
@whatthejeff
Collaborator

@sebastianbergmann, PR #862 relaxes the version requirements in a way that should prevent dependency issues. Seems like a nice compromise for now. What do you think?

@whatthejeff whatthejeff added a commit to whatthejeff/phpunit that referenced this issue Mar 14, 2013
@whatthejeff whatthejeff Fixes #860
 * Relax version requirements for `symfony/yaml`.
1a382d3
@whatthejeff
Collaborator

Seems there is also an issue with the autoloader from Symfony Yaml 2.2.0 (see #846 (comment)). I will update #862 to address the issue.

@edorian
Collaborator

@whatthejeff Currently my favorite approach would be to use namespace rewrite with yaml instead of maintaining a yaml parser (easy for tap.. for all i care this can go out of core anyways) but hard for dbunit.

But for now and since you alreay did the work on the testing I'd say lets just merge #860. If you agree

@whatthejeff
Collaborator

@edorian, thanks for the feedback. I'm going to wait for @sebastianbergmann to merge #862 as he has more insight into the PEAR side of things than I do. I do believe it's the best possible fix for the time being as it allows people to use any version of symfony/yaml, requires the least amount of modification, and addresses symfony/symfony#7385.

@whatthejeff whatthejeff added a commit that closed this issue Mar 19, 2013
@whatthejeff whatthejeff Fixes #860
 * Relax version requirements for `symfony/yaml`.
50d64e4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment