I'm not familiar with any of the details, but the testbot is rejecting patches for a contrib module which has no tests1, seemingly on account of not finding any tests?!
This forces a "Needs review" patch back to "Needs work", which is surely unwarranted.
The two errors I see in the output are:
* ERROR: No valid tests were specified.
* ERROR: Step ?Publish JUnit test result report? failed: None of the test reports contained any result
Related links are:
* https://www.drupal.org/node/1682312#comment-10723618
* https://www.drupal.org/pift-ci-job/131238
* https://dispatcher.drupalci.org/job/default/60731/console
1 grep -i test
turned up nothing of note, at any rate.
Comments
Comment #2
oksana-c CreditAttribution: oksana-c as a volunteer commentedI'm experiencing the same thing.
It would be great to get more info on why patches with no tests are "doomed to fail".
Comment #3
DamienMcKennaThis happens with other test parameters too, e.g. PHP 5.6 & MySQL 5.5 as seen in #1686588: Use From address properly.
Comment #4
DamienMcKennaComment #5
DamienMcKennaFYI I've added a number of related issues where this drupalci problem was causing the tests to fail.
Comment #6
DamienMcKennaComment #7
DamienMcKennaComment #8
DamienMcKennaComment #9
DamienMcKennaComment #10
jweowu CreditAttribution: jweowu commentedComment #11
oriol_e9gBump to Major. This is blocking contrib patches.
Comment #12
justinstandring CreditAttribution: justinstandring commentedComment #13
DamienMcKennaComment #14
drummThe test runner will need to exit cleanly with JUnit XML showing either 0 tests run or 1 fake token test succeeded. See
src/DrupalCI/Plugin/BuildSteps/setup/Patch.php
for how this is handled for patches which don't apply.In the meantime, contrib projects without tests probably shouldn't have automated testing enabled.
Comment #15
Mile23+1 on what @drumm says: If you don't have tests and don't intend to, you should turn off automated testing.
But... If you have a patch that's adding tests you'd want a fail if that test wasn't found by the testbot.
So I think this is expected behavior, though obviously up for debate.
Comment #16
jweowu CreditAttribution: jweowu commentedI presume the testbot also does basic syntax/lint checks, and #1299710: [meta] Automate the coding-standards part of patch review means it will do Drupal coding standards checks as well. So even in the absence of specific tests, I presume projects will still benefit from these generic tests?
Comment #17
DamienMcKennaThe problem is that this was an unannounced infrastructure change, thus a regression. While most projects don't have tests, the old testbot processing a patch file at least let maintainers know two things: 1. does the patch apply, 2. were there any PHP syntax errors in the code after the patch was applied. This was very useful for many projects.
Could someone please indicate what project the appropriate code is in so that someone (else) might be able to work on this? And please don't mark this "won't fix". :) Thanks.
Comment #18
Mile23There are some blockers for this, listed here: #2654650: [meta] Jobs for linting and coding standards
Comment #19
jthorson CreditAttribution: jthorson commentedWhile we do want to ensure that the other tests are available when there are no simpletest tests, I'd encourage us to keep this issue focused on the Simpletest job type when there are no tests present.
This flags one of my chief concerns with a few of the decisions made while DrupalCI was rolled out on Drupal.org, as we repeated some of the architectural mistakes that we should have been lessons learned from years of using the PIFT/PIFR architecture. Chief amongst these is the lumping of all patching/review/testing activities into a single process, which is then executed for each patch on each project posted to d.o ... the vision for DrupalCI was that each of these activities could be split out into it's own job type, so that 'quick and simple' tasks such as patching, linting, and codesniffer reviews could occur on small lightweight containers which are optimized for these tasks; while larger heavy tasks such as Simpletest testing would be pushed off to the slower and larger apache containers (which, over time, would be phased out as we move over to PHPUnit testing). The value of executing these as individual jobs instead of lumping them in to a single larger build process is the instant and asynchronous availability of results ... we should never have to wait for the execution of a 90k assertion simpletest test suite before finding out whether the linting or coding standards checks passed. Architecting these in such a way that they are separate jobs, and designing the d.o integration to support multiple different job runs for each patch, allows for much more 'as it happens' feedback to drupal.org on a per-job type basis.
Some may argue that this required more containers, and thus results in a increased resource load and duplication of effort on the testing infrastructure ... but this only holds under the assumption that we spin up a new container for each job type and patch. This is how the initial DrupalCI development did things, but only as a convenience ... my end vision was always for more lightweight 'utility' job containers which could handle the quick and simple jobs, with the current 'container per patch' simpletest approach and dedicated apache web containers being more the exception than the rule.
I wanted to provide the context of these last two paragraphs to help explain my motivation for moving this back to the original title ... if we can nudge DrupalCI a little closer to this model, the 'simpletest with no tests' and the 'run linting even when no simpletest tests' become two distinct issues.
Comment #20
Mile23So I think this issue is where people are complaining about a behavior change in the testbot.
Here's an issue dealing with some implementation details about how to change it back in the test runner: #2578527: If there are no tests found, drupalci should respond with a pass 0 tests found.
Not much yet... But yah.
Comment #21
DamienMcKenna@jthorson: Now that DrupalCI is in place there shouldn't be anything stopping a refactoring of the architecture to cover the scenarios and suggestions you mention.
Comment #22
DamienMcKennaComment #23
Mile23Well, there's plenty to *do*. :-)
Comment #24
DamienMcKennaComment #25
DamienMcKennaComment #26
DamienMcKennaComment #27
DamienMcKennaNow that the SMTP module has some tests I've removed them from the "related issues" list.
Comment #28
drummThanks. I think between #14 (what sort of change to make) and #19 (where/how to make the change) and #2654650: [meta] Jobs for linting and coding standards, I think there is a good starting point for someone to work on this.
I agree this is a regression. We moved forward with DrupalCI deployment without this because using QA.Drupal.org to check if patches apply wasn’t an intended feature, as far as we knew. It is something DrupalCI should handle cleanly in the future, especially as new job types do more than the same old testing. We don’t need to know all the issues this is affecting.
Comment #29
drummI meant to link to #2578527: If there are no tests found, drupalci should respond with a pass 0 tests found. instead of #2654650: [meta] Jobs for linting and coding standards
Comment #30
alexpottI agree with everyone who says that DrupalCI should not fail if no simpletests are present.
Comment #31
DamienMcKennaMore issues, this time for Revisioning.
Comment #32
hanoiiFor what it's worth, although probably the fix for one is the fix for all, another issue with this.
Comment #33
jthorson CreditAttribution: jthorson commentedWell ... as a first step, #2655718: Create the BuildResults has been waiting for a while.
Comment #34
MixologicComment #35
MixologicThis has now been fixed, and projects with no tests will now return a "No tests Found" and be green if the build is successful e.g.: https://www.drupal.org/node/2697717/qa