Exclude test files from release packages

Hear, hear! An additional consideration is that many of the test fixtures are compressed files, so proportionally they make up even more of the tarball (for downloads) than they do space on disk. This needlessly impacts download times, Composer install times, d.o hosting charges, folks on metered connections, etc...

And in terms of files on disk, this has the potential to make so many things more efficient, including IDE indexing or any other operation that recurses through the directory tree.

I agree that it's worth considering potentially unintended consequences. I've almost never run Drupal's core test suites locally except for development on Drupal core itself. I can't imagine people do that regularly, and if they do, they could always install Drupal from source (trivial to do with Composer). But it would be worth hearing from the community.

This will affect whether people who download/compose/clone Drupal core can run tests, so tagging for product, framework, and release management review.

As I understand it, the impact is this:

1. People who download Drupal from a tar or zip file (e.g., from the big blue buttons on https://www.drupal.org/project/drupal) won't be able to run Drupal's tests.
2. People who get Drupal via git clone (e.g., from the instructions on https://www.drupal.org/project/drupal/git-instructions) will be able to run Drupal's tests.
3. People who get a tagged release of Drupal via Composer (e.g., from https://github.com/drupal-composer/drupal-project), without explicitly asking for --prefer-source, won't be able to run Drupal's tests.
4. People who use Composer and either explicitly set the --prefer-source option, or get a branch tip (not a tagged release) of Drupal, will be able to run Drupal's tests.

Is all that accurate?

As far as I know, that should all be accurate. I’d recommend double checking a contrib project and/or an alpha release using this this technique to triple check. As mentioned in the issue summary, unintended consequences are possible, I can’t think of any offhand. This would also apply to -dev releases’ tar/zip archives.

+1 to minify tarballs, in related issue probably something could be useful

I’d recommend double checking a contrib project and/or an alpha release using this this technique to triple check

We recently implemented this for the Lightning distribution, you can verify that it works as expected by checking the latest dev release and seeing that it excludes all of Lightning's tests: https://www.drupal.org/project/lightning/releases/8.x-4.x-dev

I've also documented this here: https://www.drupal.org/node/1068944#exclude

I think #5 is acceptable but it'll definitely need a change record, could also see arguments for postponing the change until 9.x. Also not-untagging so other framework/release people can chime in.

I think this makes sense, I was wondering, though, whether it makes sense to also exclude core/modules/simpletest? Can be a follow-up, though, don't want to hold this up, just putting it out there.

The proposal outlined in #5 makes sense to me. I can think of very little times it would come up that you'd want to run tests off a tarball and in the event that it did, a git clone is just a command away...

The more barriers we can reduce to a new user getting started with Drupal, the better! :)

One thing that this might impact is people who have patches to core built into their deploy/build process, either via drush make or using cweagans/composer-patches. Most patches apply to both the code and tests, so they wouldnt apply to a stripped down version.

It's possible we may need to offer both versions.

Re: #11: One of the stretch goals of #2866082: [Plan] Roadmap for Simpletest is to make all of the test infrastructure into a subtree split library which can be optional. You'd require-dev it in your composer.json file, or remove it as you wished.

This would very likely still break people's patches if they're patching that infrastructure.

I agree that this could be a breaking change for folks patching core, but this is mitigated by the fact that it's almost trivial to switch to source packages to fix it (composer --prefer-source).

We offer a .zip file that is a 27 MB download and works everywhere. We also offer a .tar.gz file that cannot be used everywhere but is only 17 MB. Removing all directories labelled tests and Tests gives us the chance to offer 13 MB download for those people who are hit with big download costs.

I don't think the following points are good impacts:

3. People who get a tagged release of Drupal via Composer (e.g., from https://github.com/drupal-composer/drupal-project), without explicitly asking for --prefer-source, won't be able to run Drupal's tests.
4. People who use Composer and either explicitly set the --prefer-source option, or get a branch tip (not a tagged release) of Drupal, will be able to run Drupal's tests.

Forcing people that used composer to add the --prefer-source is adding another Drupal specific way to handle project compare to others PHP projects (like Symfony for instance).

--prefer-source is slow: it has to git clone and it is not cached to my knowledge.
Whereas --prefer-dist is faster: it has one ZIP to download and it has local caching.

Furthermore, not providing the tests per default (like all others PHP projects do?) will not encourage people to add tests to their projects. (Promote best practice is important IMHO)

IMHO it may be OK for people using the tarball directly but not the ones using composer.

It is not an argument but Symfony tried this solution and they reverted it later (https://github.com/symfony/symfony/issues/6605). Some links that could be interesting to read: https://github.com/symfony/symfony/pull/32674, https://github.com/symfony/symfony/issues/17749 and all the others linked issues.
I think the comment https://github.com/symfony/symfony/issues/17749#issuecomment-192172162 has some interesting points like :

You can grow the list of pro and contra arguments, the truth is that everyone is right: this is a matter of personal preference.

There's a meta argument that makes this more than just a matter of preference though. If you prefer to have test sources after Drupal removes them, you have a perfectly usable workaround (--prefer-source). If you prefer not to and Drupal doesn't remove them, you're SOL. You just have to eat the wasted bandwidth and time spent downloading a tarball that's twice as big as necessary. Collectively, we're probably talking about petabytes of wasted bandwidth and days of wasted developer time here.

Furthermore, I suspect the vast, vast majority of users don't want or need the built-in tests, and those that do are a more vocal minority. That's not to say that their opinion doesn't matter, I'm just saying we should think about the numbers on this. This is probably especially true for first-time downloaders, who are also going to be more sensitive to how much time/bandwidth it takes to just get started with Drupal.

Seems like this is still actively in discussion.

With my Security Team hat on I will say that the security angle here is quite important. There are definitely cases, both theoretical and concrete, where having test files included has led to varying levels of site compromise.

More generally: I agree with the logic in #20 that the people most likely to want the test files are also the people 1) most likely to get them already or 2) most able to figure out how to get them. Perhaps there are a few documentation pages that could be updated if this change goes in. Having a list of those in the issue summary would likely make it easier to accept this change. The list of "remaining tasks" also looks like it needs an update.

If composer patches does in fact already gather the git clones, then thats a good start to alleviating my concerns. What Im reluctant to do is force any user who needs to have a patch (like, every distribution customer out there) to have to use --prefer-source for their build needs.

D.o. is optimized for serving static files like the tarballs as dists, additionally, that is made even better for the end users with our CDN with its geographic distribution. If we have to switch a lot of users over to git clones, thats a *major* additional tax on our infrastructure, *particularly* on update days where there are security releases.

So if people can patch without forcing composer to download all of their dependencies via git clones, then Im much less concerned.

But 'the people who need test files' is a far, far greater number than 'people who plan on running tests', and is the relevant metric here.

There are definitely cases, both theoretical and concrete, where having test files included has led to varying levels of site compromise.

Could you share some cases with us please? I'm interesting in knowing more about this subject.

Personally, I think the tests should stay when you use composer to get Drupal. Removing them for production is a deploy task.

However, removing them from downloadable releases seems like a good decision. What about just using something else than git-archive to create the releases to be able to remove the tests folder?

Could you share some cases with us please? I'm interesting in knowing more about this subject.

Yes, CVE-2017-9841 is a situation of test files having a remote code execution vulnerability. Basically the worst kind of vulnerability. I have a vague memory of at least 1 xss issue in the past that I can't remember in any more detail.

Kewlio. https://nvd.nist.gov/vuln/detail/CVE-2017-9841

It says "Util/PHP/eval-stdin.php in PHPUnit before 4.8.28 and 5.x before 5.6.3" which doesn't apply to us as of core 8.8.x. But still, who knows what else lurks in your exposed vendor directory.

Previous mitigation: #2508591: vendor/ is web accessible

Current efforts from Composer intiative: #3057094: Add Composer vendor/ hardening plugin to core

I think this issue is more about core's tests that live in core/tests/.

Yes, I mention that as an example of a group of issues that can crop up when development/test code is included in the code that people run on their websites. We have mitigations for it that are imperfect and we can improve those. But this is issue proposes a good step toward avoiding the class of problems.

#2946280-8: eval-stdin.php flagged as malware Gives a different but closely related example of test files in webroot causing a site to be abused for spam.

By the way, in case anyone doesn't know, per #17, Symfony, and some of our other vendor libraries, do not remove their tests from their tarballs / zip files, but we remove those libraries' tests from Drupal core via these lines in Drupal's root composer.json file:

"post-package-install": "Drupal\\Core\\Composer\\Composer::vendorTestCodeCleanup",
"post-package-update": "Drupal\\Core\\Composer\\Composer::vendorTestCodeCleanup",

That still means that bandwidth is being wasted on downloading Symfony (and other) test files when installing Drupal from composer, but at least those files don't end up on Drupal production sites or in Drupal tarballs.

Not sure if that's helpful or sparks any ideas; just sharing how we as a consumer of Symfony are dealing with Symfony's choice to includes test files in their release packages.

composer patches doesn't enforce git clones, it it just a mechanism for downloading a patch and applying it to whatever composer download, whether it's from a tarball or git.

So this would mean that if you're using a core patch from Drupal.org, it would cease to apply to the dist version of core - you'd have to modify the patch to remove all the hunks that touch test files. As someone who uses composer patches a lot, this would be very inconvenient.

Also composer patches has nothing to do with core development, it's just a way to manage patches for a development or production site, so anyone who has a composer-based site and finds they need a core or contrib patch would run into this, not only people working on the patches.

@catch That's been discussed a bit. See #5 point #4, comment #18, and comment #20. I'm not sure that composer-patches does prefer-source on the target package by default as has been stated here. If it does then that seems to really mitigate that concern. It would be good to definitively verify that.

I think the biggest blocker to this is the concern from #22 that this could raise load on d.o infrastructure.

Just a note: Symfony decided to remove tests from dist version of their packages in https://github.com/symfony/symfony/pull/33579

Regarding the question of whether composer patches does prefer-source, I have done a test with webforms, see https://www.drupal.org/project/webform/issues/3093934#comment-13350022. Instructions are in the referenced webforms comment if you wish to experiment for yourself.

The conclusion I found:

If tests are excluded using a gitattributes file, and you want to apply a patch using composer patches that has changes to a file in the tests directory, then it will fail, unless you specified to install the package from source.

I can see people have presented several goals of this issue:

1. Security
2. Reduce disk space on local server
3. Reduce download size, hence reduce the load on Drupal.org servers
4. It's common to apply patches even on live sites so it must still be possible to apply patches that might make changes to test code

Perhaps the comments haven't stated it clearly yet but it seems that if we fail to achieve (4) it is a disaster. People will be forced to prefer source and then we don't achieve any of the goals plus as explained in #22, prefer source is much worse for the server load.

Therefore it's seems that we have to find a way for patching to work with test files missing. I'm no composer expert, but it feels like this should be feasible. Looking at cweagans/composer-patches, it seems to use git apply which has an --exclude option. So could we create an enhanced version of cweagans/composer-patches that reads .gitattributes if present and passes the values through to git?

Thoughts anyone??

Great thoughts @AdamPS! Regarding:

> So could we create an enhanced version of cweagans/composer-patches that reads .gitattributes if present and passes the values through to git?

I really like that idea and on the surface it seems possible. A quick look at https://github.com/cweagans/composer-patches/blob/master/src/Plugin/Patc... the getAndApplyPatch() method seems to handle it. It looks like there's a fallback to patch if git apply fails, and I don't see an exclude option with patch in the man page. Let's assume we could make something work for a moment and ask a higher level question:

Is there a case where you would want composer patches to apply a patch to a directory or file that is specified in the .gitattributes file as export-ignore?

Chiming in about tests. I don't mind if core's tests are removed. But, I extend core's base test classes and need the test suite definition.

+++ b/.gitattributes
@@ -59,3 +59,10 @@
+core/tests                      export-ignore

Change this to the following and I'd be happy. Otherwise all of my tests and TDD approach to work would be ruined. Yes, even client work.

/core/tests/Drupal
/core/tests/fixtures

EDIT: nope, that wouldn't work.

See web/core/tests/Drupal/KernelTests/Core/Entity/EntityKernelTestBase.php

That would get excluded.

EDIT 2: what about module test base classes? I extend Migrate's test base for our various import tests.

Per my thoughts in #37 I'm kicking to needs work. This would completely break all of my projects as PHPUnit would be broken -- no test suites, no base test classes.

So right now, we have the vendor hardening Composer plugin. You can read about it here: https://www.drupal.org/docs/develop/using-composer/using-drupals-vendor-...

I haven't tried, but it might be configured to remove test directories from the drupal/core and drupal/[core-module] and drupal/[module] packages.

I'm not entirely sure if it will find the proper directories, given that those packages are installed using composer/installers. If not, we can make a child issue here to allow it to do that.

We'd also need a policy on whether our Composer project templates would include this configuration by default. That would allow the workaround of just removing that config if you need the test stuff, or alternately creating your own Composer project template pre-configured the way you like it.

I still think that the best and most useful solution is to do all the things required to make all the different core testing frameworks into their own packages. Then, instead of worrying about this configuration, you could just say composer require --dev drupal/core-build-test-framework drupal/core-run-tests-runner, and for production: composer install --no-dev.

And, to top it all off: If you use drupal/recommended-project to build your Drupal site with a docroot that does not contain vendor, you're much better off in this regard in the first place.

.gitattributes support seems like a great thing to have, especially if it means not breaking everyone's workflow, so I wouldn't be opposed to making git apply the default and only method for applying patches if that helps.

For the same functionality with patch, you'd have to use something like filterdiff. I'm not sure how widely filterdiff is installed though. Anecdotally, my macOS machine has patch but not filterdiff, so this functionality wouldn't work out of the box for me. I suppose composer-patches could try the fallback only if both patch and filterdiff are installed, and then try to parse .gitattributes or some config value in composer.json to control which hunks get stripped out.

Alternatively, composer-patches could have some kind of preprocess step for patches where an external plugin could have a chance to modify the patches before they're applied. This could be another plugin that Drupal core (and Symfony projects) require, and all it would need to do is strip out hunks related to test files.

Anyway, happy to help. Let me know which way makes more sense for Drupal. This is a pretty good time to make changes like that since 2.0.0 isn't out the door yet. Breaking changes to composer-patches are definitely possible at this point.

OK I updated the IS to include recent comments.

Work happening in #3108262: Support directories other than vendor in the Hardening Plugin

This would allow users to configure which core directories to remove during composer install.

Besides actual tests those directory contain Drupal testing framework (base classes, traits, helper modules, fixtures, etc.). So that straightforward excluding tests directories from drupal/core package will break tests in custom and contrib projects won't it?

Distinguishing Drupal Core tests and Drupal testing framework based on file paths could be tricky because they are closely intertwined.

Some other benefits of not having tests in the project code base by default.

• IDE will index a project faster
• Fewer irrelevant references in IDE autocompletion
• Fewer irrelevant references in grep output
• Faster `git clone` on non-composer projects

Added #14 as "Alternative resolution 3".

Separating the underlying code from the actual tests sounds like a good move no matter what the other plans are. Similar to separating code and data. Separation of concern and all that good design stuff. Given that PHPUnit is the future, does PHPUnit have a structure/convention we could use for Simpletest?

The majority of comments here are from people who develop PHP code. The majority of Web sites are built by people who only develop themes. Twig was added to separate theme development from all the template code, removing the need to test PHP code after theme changes. And then there are all the sites installed through Sympatico style host scripts with no option to change code. A DruLite download is aimed at people not developing PHP code.

Hello,

I have recently read this article https://php.watch/articles/composer-gitattributes, and now knowing the export-ignore feature from .git attributes I asked if something similar was possible/recommended for Drupal projects. @andypost gave me a link to this issue on Slack (thanks again).

I have read the issue summary and all the comments. I don't know what is the trend. So if this can help, this is my opinion and usage of Drupal as a developer and contributor.

Usage:

I use Composer on almost every Drupal projects I do or am involved in, only a very few special/small missions are not using Composer.

I have patches applied with cweagan/composer-patches on (almost) every projects on core or other contrib projects.

I prefer using tagged releases. Only when a contrib project does not have a tagged version or some patches are impossible to apply and a fix had been commited in dev version, then I use a dev version and still using a commit hash in composer.json to ensure to not move involuntarily with a composer update. Also tagged releases allows to download translations.

For client/personal projects, even when not doing automated tests, I would like to be able to see the tests because sometimes it helps to see how things work (JSON:API tests are really helpful) or to have real examples on how to test some particular stuff.

In my local setup, I have a dedicated stack/skeleton like the one used for client projects, but this one is for contribution.

And I have a "contrib" folder at the root of this setup where I clone the projects I want/need to make/update patch for (Drupal core, Drush, Composer plugins, any Drupal contrib projects).

So when I need to test stuff, I rsync code from this "contrib" into the appropriate place (vendor for Composer plugins, app/core for Drupal core, app/modules/contrib for ... etc.). That way I am sure that in any case Composer will not remove the cloned repositories I have and all my local work that could be lost.

Opinion:

I don't want to have to use a dev version in Composer (and all the cloning slowness associated) to see the tests or be able to execute it.

I completely agree with the opinion that people downloading Drupal using tar/zip archives are not people interested in tests. So not having it in those archives is perfectly ok IMHO.

I also agree that tests code should be removed for production environments (primary topic I searched this subject for). And if this will be too complicated with .gitattributes or other stuff, it can be delegated to package building tools per project at the preference of the team on the project. Even if, in that case, the network gains for Drupal.org infrastructure will not be present.

That's why I think the approach 3, a require-dev package containing Core tests is the best. I don't know if this could be extended (automatically) for other contrib projects.

Also I may be missing some aspects of the problem.

That's my 2 cents.

Cheers,

I'm in favor of more subtree splits, structured as follows:

Each testing framework becomes a component.

drupal/drupal gets a path repo for each testing framework.

Frameworks are required in drupal/core's require-dev.

The vendor hardening config for drupal/core is updated to delete tests (but not frameworks, since they are now components).

The vendor hardening plugin is updated to understand the difference between require and require-dev. That is, if you say composer install --no-dev then the hardening occurs, but if you say composer install then it does not. This is maybe not the perfect solution, because it means we don't default to security.

Proposal #1 seems good to me - it's completely reasonable to require people running tests to use prefer-source.

Composer patches was brought up, but now that we're using MR diffs on d.o everywhere, it's not recommended to link to the online MR diff anyway for security reasons, instead people should commit a local copy of the patch (or use one uploaded to the issue in some cases). When preparing that patch, it's fine to delete any changes to test files. This also makes it more likely that the patch will apply longer if unrelated changes to test coverage are committed to the project too, sometimes I do it anyway for that reason, especially backporting something for a previous minor/major release.

> Composer patches was brought up, but now that we're using MR diffs on d.o everywhere, it's not recommended to link to the online MR diff anyway for security reasons, instead people should commit a local copy of the patch (or use one uploaded to the issue in some cases). When preparing that patch, it's fine to delete any changes to test files.

That's still going to be fairly fiddly to do.

You're either manually poking around in a .patch file removing bits.

Or you need the git skills to revert portions of a branch, and that's fiddly too -- something like:

> git checkout 11.x -- */test/* [maybe? will wildcards work here?]

and then commit the result to your own local branch. But then if the original MR is updated, you have to repeat that process all over again.

I mean, sure, I often have local branches in my clone of core for backporting MRs to a stable version. But I'm not sure we can expect everybody to do that.

The best bet would be a preprocessor to composer-patches, as has been suggested, but I don't know how close a 2.0 release is of that.

I'm assuming a stance that if people want to use patches of unsupported (non-released) changes in their own project they can be expected to make their own patches. In that case, it's built in git functionality to to exclude files with git's powerful path matching syntax. That works in diff, log and format-patch to name a few of the commands that use the same underlying features.

As an example for a recent Drupal core commit (using --stat to limit the output to what's relevant for the discussion):

$ git diff --stat 422099995ecfe78f1e29248374fc05cd4a556c1d^!   

core/modules/package_manager/src/ExecutableFinder.php                | 22 ++++++++++++++++++++--
core/modules/package_manager/tests/src/Unit/ExecutableFinderTest.php | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 58 insertions(+), 2 deletions(-)

$ git diff --stat 422099995ecfe78f1e29248374fc05cd4a556c1d^! -- ':!**/tests/**'
core/modules/package_manager/src/ExecutableFinder.php | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)

Notice how the second diff doesn't include the test files and could be used to create a patch that would work.

There's also the issue that including test changes in the diff increases the likelihood that the patch won't apply, either in the first place because there are conflicts with the older branch already, or if it's not committed anywhere yet, that test conflicts are introduced in minor versions even when the runtime code change itself would continue to apply. So there are benefits to removing test files already (I often do it if I'm doing a local backport of a core MR diff already partly for this reason).

For me this is fantastic idea which we should definitely do. But given the disruption to patches I'd only do this in a major release so minor updates to sites that have patches applied don't break just because we've removed tests.

I bumped this because of #3533965: Package Manager actions intermittently blocked on shared hosting but also now think #3534109: Permanently maintain a sandbox directory in package_manager is probably more urgent for package_manager/Drupal CMS. If the other issue works, then this issue would be a very nice to have on top (and for the other reasons it would be good), and starting in Drupal 12 seems reasonable, although if we somehow miss Drupal 12 then I'm not sure I'd want to wait until Drupal 13, could be Drupal 12.1 with a PSA since it's still early in the major release cycle.

Bumping this to critical and tagging with 'Drupal CMS release target' - we're fairly sure that doing this issue will significantly improve (although not necessarily fix) the problem of package manager sandbox creation hitting shared hosting abuse limits. If we have less files to copy around, there are... less files to copy around.

I'm also removing the RM and FM tags because I discussed this with @alexpott and @xjm at DrupalCon and they were both +1.

The actual commit would need to happen again 12.x when it opens, but we can have something ready to go before then.

edit: thanks to @pameela for demoing how this goes wrong at DrupalCon - entire shared hosting account got locked down and it requires a support ticket to get it unblocked, not even a rate limit, full account freeze.

Hmm I think this is probably the right tag for Drupal CMS tracking?

For #37/#38, I think if we try to be clever about which test files to remove it will be more or less impossible to finish.

However given we're only going to start this in a major release, we can probably do one of the following:

1. For running tests locally, or CI, require sites to use --prefer-source prior to running tests.

2. Have another (probably new) package like core-with-tests that doesn't exclude them - but only if #1 is a non-starter.

We also need to make sure this will work for contrib and gitlab.

I was debating to myself whether this is something we should do only in a major versus whether it is allowable in a minor. Technically, there is an argument that this is partly allowable in a minor release, because neither tests nor file locations are public API. So long as the autoloader (and GitLab) can still properly serve the testing framework and its base classes and traits for CI, we could do parts of this in 11.3.

This does make it more disruptive than simply the files not being in the packages:

It's common to apply patches even on live sites. Patches often contain changes to test code and would by default fail if it is missing (see #33. Therefore as a dependency of this issue, we need an enhancement to the patching mechanism to allow applying patches when test code is missing - see this issue.

For me, the value of unblocking Package Manager is still worth breaking almost every composer-patches in a minor release, and we should consider whether this could be committed before 11.3.0-beta1.

In practice, we are very close to 11.3.0-beta1 (about 3 weeks away), and the opening of 12.0.x is the next thing to happen after that anyway. Making it a major-only change might be nicer for site owners (or anyone with weird CI/CD that relies on release packages), and I can't think of any practical reason to offer parity on the maintenance minor branch for the change. It's not really something that would need to be part of the continuous upgrade path.

If there are any clearly minor-safe blockers, we should definitely try to land those for 11.3.

The first one!

Reparenting. I've listed it on both metas because we should do this in 12.0.x basically as soon as the branch opens, but we must do this prior to 12.0.0-beta1.

Adding the 12.0.0 release priority tag because we're likely to use that for other issues too.

Reviewing the use of 'release priority' and 'release target' I think the intention is that this is a 'target'. 'Priority' has been used for things not directly important for the version about to be released. I've seen it used for a broader range of issue, such as long term strategy issues. Of course, there sometimes a mix but that is the trend I see. 'Target' is more urgent than the 'priority' ones.

Discussed tags in committer slack and updating based on that discussion

To me, proposal 3 is the best solution, because it solves the problem while remaining a trivial change for users who want to include tests (composer require --dev drupal/core-tests) but which also reduces the risks on DA infrastructure (git clones are much more expensive than zip file downloads)

However I don't think it solves the patches problem unless composer/installers or the scaffold plugin also can be convinced to reconstruct the original structure once drupal/core-tests is installed.

I had a quick look at the subtree split tool that is currently used for components - https://bitbucket.org/drupalorg-infrastructure/subtree-splitter which in turn mostly depends on https://github.com/splitsh/lite

splitsh-lite does actually support multiple prefixes in the split so it seems technically possible to find all tests directories, include them in the drupal/core-tests split, and exclude them from drupal/core. At the moment subtree-splitter doesn't support anything close to this but it seems like it would at least be possible if anyone wants to attempt it.

I basically share the same concerns as #37 regarding test infrastructure. If the subtree split is too much effort, maybe we can use .gitattributes to only exclude **/*Test.php (concrete test classes) and core/modules/*/tests/modules (test modules)? This would leave behind all other test infrastructure - base classes, traits, etc. - which might be enough for most custom/contrib tests to live with?

Opened an MR with the idea from #79.

Add me to the list of people concerned about this I guess. In regards to test modules, something like entity_test is almost an API with lots of contrib using entity types defined by it and we even provided BC for some functions it includes. Another example is redis, which extends a number of core tests. I'm aware of the BC risks, but it allows me to ensure that the altenative implementations of the lock, queue and similar systems are compatible with core expectations, also as core makes changes.

Just throwing in a random idea, didn't read all of this. What if drupal/core-tests (or drupal/core-with-tests then) wouldn't be *just* the tests that would need to be spliced into their original places, but the whole package and comes with an instruction that it replaces drupal/core? That should continue to work with anything that requires drupal/core.

What if drupal/core-tests (or drupal/core-with-tests then) wouldn't be *just* the tests that would need to be spliced into their original places, but the whole package and comes with an instruction that it replaces drupal/core?

I think this would be the simplest thing if we could make it work. That way we can remove as much as possible from drupal/core, but provide it all again.

Not sure what the composer command would look like to switch between the two on CI though, would be great if just specifying drupal/core-with-tests in require-dev was enough.

If we could do #82 with drupal/core-dev then maybe this is achievable with no changes at all for most users!

Just another data point.. the WSE module also heavily relies on extending core test classes for checking the same assertions in the context of a workspace :)

I believe that approach #1 is still the best one - core and every contrib project declare what should be omitted from export by Git. As Git export is used only to produce artifacts for distribution, e.g. for running in production, that is how it should be.

And in my opinion using --prefer-source for CI testing is fine. Composer cache can help to speed up process.

The one degraded experience is the patching and is important of course , but it has a workaround - manual patch amendment to remove test data from the patch, and I consider this lesser evil than delivering all the test data to production environments that is happening right now.

Probably it's a good idea to exclude the docs from the module packaging too. For example, the AI module in the "docs" directory here https://git.drupalcode.org/project/ai/-/tree/2.0.x/docs?ref_type=heads provides 170+ files that are usually not needed for end users at all. Actually, they are required to only generate the documentation pages here https://project.pages.drupalcode.org/ai/

Probably it's a good idea to exclude the docs from the module packaging too

Make sense, actually all my projects on D.o has similar extra configuration in .gitattributes: https://git.drupalcode.org/project/easy_encryption/-/blob/1.0.0-rc7/.git...

@catch @berdir composer has a notion of a package replacing another, done in the package's own composer.json file

https://getcomposer.org/doc/04-schema.md#replace

Could we have drupal/core-with-tests required by CI and that would just use replace for Drupal/core?

@acbramley yes I think this would be best if we can make it work.

We should add the latest idea of a core-with-tesrs to the issue summary and also try to implement it. Moving to needs work for that.