Reverting the removal of the tests via .gitattributes

[fabpot]

I think that it is not worth removing the tests classes from archives via the .gitattributes file. The more I think about it, the more I think that the benefits are really small and the drawbacks quite huge:

• people use some of our test classes in their own tests (we have already moved some of them to a Test/ directory and that helps bu I fear that many more will need to be moved -- see https://twitter.com/huizendveld/status/288331330998116352 for an example);
• people like to have a look at the tests to understand how the code works;
• when working on a patch locally on a project, there is no way to run the unit tests to check that everything still work as expected.

I would like to revert this change. Opinions?

[fabpot]

Original pull request: #5674

[fabpot]

And the issue that started it: #6047

[clemherreman]

Same as @marijn, tests are part of the documentation, and have proved more than useful to understand how internals are working. Also I like to run the tests when putting my application on production, just to ensure that my server environnement doesn't break them.

[marekkalnik]

I totally support this. Moreover I think that the test base of symfony is great and we need to encourage people to learn from it rather than hiding it. 👍

[igorw]

To clarify the intent of the original PR: The idea is make the stable, zipped packages optimized for deployment. That means stripping out things like docs and tests that are not needed in production. When this is applied on a large scale, the savings are significant.

Of course stable packages are also used in development, and as such having the documentation present can be very useful.

For the record, I have no problem reverting this. Stable packages != production; stripping out useless things in the deployment process is a sane approach. The fact that composer caches packages locally also helps for speed and bandwidth.

👍

[Seldaek]

I tend to agree it probably causes more pain than good. I fixed composer docs to avoid promoting the practice, because I have seen it spread quite a bit lately composer/composer@bf32869

[ghost]

@fabpot - I think it's all about the target audience. If you are deploying a project, the tests are not necessary. if you are working in a development environment, then you certainly want the tests but you'd get them anyway with Composer which you'd sure be using when developing a project...

[ghost]

Thinking some more, when you have to package a large project it's usually necessary do some assembly and packaging tasks anyhow, so it probably doesn't matter either way.

If tests are considered part of the documentation, then +1 for keeping them. What consumers do downstream when packaging their final products is their own prerogative.

+1 for reverting the PR.

[acasademont]

+1 for reverting this, I look a lot at the tests to see how the code works, not everything is in the docs!

[Seldaek]

One thing worth noting is that test helper classes and such that are
meant to be used by other people should really be moved to a Test/
namespace ideally, to clearly separate public API from internal tests.

[igorw]

I strongly agree with @Seldaek, the tests are not a public API and should not be depended on.

[fabpot]

I agree too with Seldaek and I won't revert the move of some test classes. If some others need to be moved, let's do it.

[fabpot]

done

[Tobion]

If people intend to run tests/look at tests when developing, they can also install vendors locally with --prefer-source and when deploying on servers with --prefer-dist.
So this way they got both advantages. Low bandwitdth when deploying and flexibility when developing.

[jdufresne]

I agree with @Tobion

If the gitattributes are restored, you get the best of both options. Use --prefer-dist if you do not want test and doc files, use --prefer-source if you do.

This seems like it would solve both side elegantly, so why not do it?

In my opinion, if you want the code is a deploy-able/library state, that is what composer is for. If you want the code in a develop-able state, that is what git clone is for.

[tgr]

I don't understand how the benefits and drawbacks were weighted here; ignoring parts of the package which are not required to use it seems like a no-brainer to me. People using them for development/CI should use --prefer-source (and people working on patches especially so; how would you even submit them otherwise?). Really the only drawback that I can see is that you have to remember to type --prefer-source for every composer install command, which is not exactly hard. On the other side, beyond the bandwidth savings and reducing exposure of the production environment, it would also help with issues like #21750.

[cordoval]

having tests to understand code argument: Tests are not going anywhere, it is only for -prefer-dist
test classes being used argument: That is a bad practice that should be discouraged, you also don't run tests on production with how current deploys are being made (docker, kubernetes etc). Environment is the same as in production than on testing environment.
developing symfony argument: People that develops symfony --prefer-source because contribs are done on git.

Besides the little probability of changing minds here i want to leave a trace to accomplish no tests on deployments one has to write a script to rm -rf Tests folder on every first level component provided one is not requiring symfony/symfony and is requiring symfony/framework-bundle instead with the use of composer replace key.

[mvrhov]

I'm removing tests and docs as part of a deploy procedure.

[nicolas-grekas]

This discussion already happened in #17749