We've seen a couple error scenarios where no tests are actually run for a branch test or patch ... occasionally, this is due to run-tests.sh not being passed any tests via the --file argument. See the related PIFT issue #1156976: 0 tests passing is not green.

We should detect this condition and return an appropriate response instead of allowing a zero-test test to run and return green.

Comments

jthorson’s picture

Status: Active » Needs review
FileSize
740 bytes

Patch attached.

jthorson’s picture

Well ... that didn't work. New approach.

rfay’s picture

jthorson’s picture

Merged this patch into the one posted at #1126112-11: Full tests of Drupal being done for some contrib patches, as they would otherwise conflict.

jthorson’s picture

Status: Needs review » Fixed

Fix committed in referenced patch.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

jthorson’s picture

Status: Closed (fixed) » Active

Patch reversed, as the 'fail' condition prevents the ability to leverage testbots to ensure a patch can be applied against a project with no tests.

jthorson’s picture

Status: Active » Needs review
FileSize
1.19 KB

New approach, which should catch 'No tests found' due to testbot errors, but will not execute run_tests.sh if no arguments are found for the '--file ' parameter.

jthorson’s picture

Status: Needs review » Needs work

After more thought, this won't actually catch the 'no tests found' error ... but is still a step in the right direction.

jthorson’s picture

Posting patch here so I can validate on scratchtestbot ...

jthorson’s picture

There ... I think this does the trick.

This patch takes the following approach:
i) Check for empty arguments on the "--file" parameter, and skip run_tests.sh if there are no tests to run
ii) Scan the review output for "PHP Fatal error", and fail the test if one is found ... thus fatal errors which interrupt the review process do not result in 'green' results.

This check could possibly be extended to include PHP Warning ... but I think the 'Fatal error' version this should catch 90% of our problem cases now.

jthorson’s picture

Status: Needs work » Needs review

Looks promising ... tests result can be found at:

zero tests passed: http://rfay.redesign.devdrupal.org/node/1073590/testing-status
PHP Fatal error: http://rfay.redesign.devdrupal.org/node/19304/testing-status (6.x-1.x)
With actual tests passed: http://rfay.redesign.devdrupal.org/node/19304/testing-status (7.x-2.x)

jthorson’s picture

Rerolled patch.

jthorson’s picture

Status: Needs review » Fixed

Committed to 6.x-2.x (264fb4f).

Couldn't find a patch which contained a FATAL but didn't break the testbot, so that component of the patch is unproven. Will need to watch for this scenario after deployment.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Updated to refer to pift issue.