[policy, no patch] Mandate all projects have Automated Testing enabled

Are you referring to testbot testing, or the code scanner? FYI - the code scanning has already been deployed to d.o and is scanning every project, I believe on every commit. It has not been made visible because the scanner needs some amount of work. I believe Michael Hess is looking for people to help with it, but don't know if there is an issue for it.

Well there's a very big difference between testbot testing and code scanner testing. The testbot relies on the module having tests written for it -- we can enable it for all projects if we like, but if maintainers don't write test cases it won't do anything!

What I'm referring to are two different things. The code scanner is something no one can see right now, and is what we will use as part of the project application process, as well as determine project health, etc for all existing projects.

You are then referring to the option of enabling testing for each project. As in, send my patches to testbot. We can discuss making that mandatory, but it only helps for tests that are written for that module (and core tests.) Testbot does not check code style, do security review, etc, that the (what I keep referring to as the) scanner does.

I feel like we're not having the same conversation. The testbot doesn't hold anyone to a standard other than one of their own choosing. Well, I guess the one advantage would be that it would have to not break core. But, otherwise, it won't enforce any kind of standard or do much to treat old timers the same as newbies. The scanner, however, will treat all projects the same and notify people of problems that it encounters.

I agree with #7 -- the testbot @DamienMcKenna seems to be talking about won't do anything much!

> I can easily create projects without enabling the testbot and release official, stable releases without any patches ever having passed a single test

The testbot you're referring to runs the test cases it finds in your module code, so if you don't write test cases, it won't have anything to test. I presume in that case, all patches will appear to pass!

However, I do completely agree with the idea behind this issue -- subject existing projects to the same automated review that project applications receive.

DamienMkKenna wrote:

One problem I see it is that new contributors are held to a different standard to people who were grandfathered in, myself included. I can easily create projects without enabling the testbot and release official, stable releases without any patches ever having passed a single test.

The automatic testing we force on new contributors is the Project Application Review (PAReview.sh) robot. This is a bash script with a passion for short lines and for using full stops to terminate comments. It has nothing do to with the Drupal Testing framework (in core for D7).

As pointed out in #5 and #7 (written while I was typing) turning on the Testing framework will have no effect on existing projects unless the maintainer has prepared test scripts that really test critical components. Any maintainer who wants to game the Testing framework can easily do so by preparing a test script that only test trivial things (or nothing).

If I read the issue summary correctly, you want all projects to have the Testing framework enabled. This is currently not a requirement for new projects. Does this means that you want to introduce a new hoop for new contributors to jump through, or are you confusing the Testing framework with the Project Application Review robot?

While having the Testing framework in place is a great help for maintainers who want to use well-written test scripts to weed out buggy patches, I don't see forcing this on new or old contributors is going to help.

Could you please clarify what "testing" you are talking about in the issue summary?

You say is #7 that you understand the difference between the testing script used as part of the application review process and simpletest / phpunit -based tests, but the issue summary just require "testing" to be turned on without stating what sort of "testing" you want turned on.

As a former maintainer of the testing system, I would strongly recommend against this proposal. As a community, we do not force contributors to a particular development model or style, and we can not assume or mandate a test-driven development methodology (no matter how much we might recommend it). To mandate enabling testing on every project assumes that every project has tests to run; which is definitely not the case.

In addition, enabling automated testing on every project would significantly increase the testbot resource load, resulting in increased infrastructure costs for the project. I'd question whether forcing the testing checkbox on for all projects would provide enough benefit to justify this extra cost.

If this proposal were re-positioned as enabling code-scanning reviews on all project, than that would be more reasonable ... but we've already got a TWG issue and various related tasks actively chasing that goal.

I'm going to recommend this be closed ... flagging for discussion at the next TWG meeting.

What's the difference between 'code scanning reviews on every project' and #1299710: [meta] Automate the coding-standards part of patch review? Or at least, a combination of #1299710: [meta] Automate the coding-standards part of patch review and on-commit or on-tag branch tests for every project by default.

catch wrote:

What's the difference between 'code scanning reviews on every project' and #1299710: [meta] Automate the coding-standards part of patch review? Or at least, a combination of #1299710: [meta] Automate the coding-standards part of patch review and on-commit or on-tag branch tests for every project by default.

Assuming 'code scanning reviews on every project' refers back to #15, I just want to note that this is not what this issue is about at all, and IMHO it is not desirable to "re-position" it in that direction.

The title of this issue is 'Mandate all projects have Automated Testing enabled'. I think this refers to the Drupal Automated Testing Framework. I.e. the OP wants to make it mandatory for maintainers to have this enabled (it is currently just an option and the default setting is "off").

The other issue: #1299710: [meta] Automate the coding-standards part of patch review - proposes to add another component (i.e. Coder Review) to the aforementioned testing framework. It is not about making such scans mandatory for every project (which IMHO would be horrible).

That's why I asked the question though.

If you have no tests written for your module, then whether automated testing is on or off will have no effect.

The three things that can happen in the short-term when there are no tests written are:

1. php lint
2. Checking if patches apply (only for patches of course)
3. DrupalCS checks once they're up and running.

Enabling testing for all projects is meaningless unless we're doing 1-3, no actual tests would get run. If a module has tests it seems very unlikely the project maintainer wouldn't enable test runs for the project so enforcing it is unlikely to make any difference for the projects that have test coverage.

Even if DrupalCS checks got run against every project and patch, they don't need to result in big red warnings or marking patches CNW, could just be informational report.

Is the testing framework able to handle automated testing for contrib projects with dependencies?

The reason I ask is that some contrib modules depend on other contrib modules. Others depend on libraries being installed that you have to download from outside because of license incompatibilities. So, if I maintain a module like that (which I do, actually), and have written some tests (which I have), and I enable automated testing, I think it would just erroneously report that all the tests fail if the testing framework cannot handle downloading the dependencies.

> Is the testing framework able to handle automated testing for contrib projects with dependencies?

Yes. And you can also indicate dependencies that are only needed for tests.

> This helped catch small problems with patches, e.g. syntax errors left in, patches created against the site root instead of the project's directory, etc, and was very useful from a project maintainer's perspective.

I can imagine that could save a fair bit of time when it adds up over all the projects and maintainers. On top of that, there's also the way that if I as a maintainer decide I have a bit of free time and I'll pick one patch to review from an issue queue, if that patch turns out to waste my time because it's not been rolled properly, I'm likely to give up completely rather than move on to another one.

RE #22, where is this documented? Is it Composer, and if so, what if the project you need is not in Composer or you have to do something funky to get the right downloads?

It's in the documentation for .info files: https://www.drupal.org/node/542202#dependencies but I don't know if it's the documentation for testing.

Thanks! I knew how to specify project dependencies in .info files... The information on test dependencies I didn't know about, so that is helpful, possibly... But that doesn't tell me how to tell the testing system things like:

a) I depend on the foo_bar_baz module, which is part of the foo_bar project on drupal.org [in other words, where to find the foo_bar_baz submodule that I depend on, when I don't depend on the base foo_bar module].

b) I depend on library xyz being downloaded from example.com/something and placed in the sites/all/libraries directory so that something.js is at location sites/all/libraries/xyz/subdirectory/something.js
etc.

I also don't think that the test_dependencies stuff is in the 8.x .info.yml file docs... let's see, those are buried at:
https://www.drupal.org/node/2000204
No, nothing there about test-only dependencies.

Based on discussion during the last two TWG meetings, I am marking this issue as 'won't fix'.

During our discussion it wasn't immediately clear what issue this proposal is attempting to alleviate. Mandating that all projects have Automated Testing enabled would provide value if all projects had tests to run; but in the absence of that, does not seem to justify the significantly increased testing infrastructure load and associated costs which would result. While the situation has improved over the last year, we still have significant gaps in the types of projects which can be tested via the automated testing infrastructure (such as 3rd party integration modules, for example); and testing these would only result in unnecessary overhead.

While the value of this proposal could be increased by requiring contrib maintainers to write tests on their projects, this approach was immediately rejected as being overly prescriptive; and the increased barrier to contribution which would result would counter any perceived gains which might be realized through such a proposal.

Enabling some sort of automated scanning and reviewing on a per-commit basis is certainly an idea that is widely supported and agreed upon within the community, and something of defacto best-practice recommendation already ... when I read the wording 'have Automated Testing Enabled', it suggests/infers 'force-checking the automated testing box on all projects'; which I do not think is the direction we want to go from an implementation perspective.

But there is certainly nobody arguing against the suggestion that syntax checking and other review tools would be a 'Good Thing to Have' on all projects.