Problem/Motivation

Sorry if this is a duplicate; it is currently impossible to search Drupal project issues for 'run-tests.sh'. Try it and see.

Follow-up to #2189345: run-tests.sh should exit with a failure code if any tests failed

run-tests.sh has incorrect docblocks and poor coding standards.

Let's fix that.

Proposed resolution

Fix it.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#13 2605290_12.patch16.22 KBsanduhrs
#5 2605290_5.patch15.89 KBsanduhrs
#4 2605290_4.patch10.91 KBMile23
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Mile23 created an issue. See original summary.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Issue summary: View changes
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Issue summary: View changes
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Active » Needs review
FileSize
2605290_4.patch10.91 KB

This patch changes all docblock coding standards errors, fixes some documentation errors, and makes very minor coding standards changes to executable code.

sanduhrs’s picture

sanduhrs
he/him
Primary language German
Location 🇪🇺 Heidelberg, Germany, Europe
CreditAttribution: sanduhrs at erdfisch commented
FileSize
2605290_5.patch15.89 KB

Reroll and expansion.
no logic changes, just formatting.

Status: Needs review » Needs work

The last submitted patch, 5: 2605290_5.patch, failed testing.

sanduhrs’s picture

sanduhrs
he/him
Primary language German
Location 🇪🇺 Heidelberg, Germany, Europe
CreditAttribution: sanduhrs at erdfisch commented

Ah, sorry this is based on #2189345: run-tests.sh should exit with a failure code if any tests failed, so the bot can't test it right now.

sanduhrs’s picture

sanduhrs
he/him
Primary language German
Location 🇪🇺 Heidelberg, Germany, Europe
CreditAttribution: sanduhrs at erdfisch commented
Status: Needs work » Needs review

#2189345: run-tests.sh should exit with a failure code if any tests failed has been commited, this patch passes now.
Please review.

The last submitted patch, 4: 2605290_4.patch, failed testing.

sanduhrs’s picture

sanduhrs
he/him
Primary language German
Location 🇪🇺 Heidelberg, Germany, Europe
CreditAttribution: sanduhrs at erdfisch commented

Hide old patch.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs review » Reviewed & tested by the community

+1.

Setting to RTBC even though it was my patch originally.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Needs work
Issue tags: -rc eligible 
  1. +++ b/core/scripts/run-tests.sh
@@ -356,18 +361,20 @@ function simpletest_script_init() {
-    list($php, ) = explode(' ', $sudo, 2);
+    list($php,) = explode(' ', $sudo, 2);

@@ -619,12 +625,15 @@ function simpletest_script_execute_batch($test_classes) {
-            list($db_prefix, ) = simpletest_last_test_get($child['test_id']);
+            list($db_prefix,) = simpletest_last_test_get($child['test_id']);

@@ -893,7 +909,7 @@ function simpletest_script_get_test_list() {
-        list($class_name, ) = explode('::', $test_class, 2);
+        list($class_name,) = explode('::', $test_class, 2);

    The comma is not required at all. See https://3v4l.org/oIa3D 

  2. +++ b/core/scripts/run-tests.sh
@@ -1109,7 +1125,8 @@ function simpletest_script_reporter_write_xml_results() {
+        // We've moved onto a new class, so write the last classes results to
+        // a file:

    A can be on the previous line

sanduhrs’s picture

sanduhrs
he/him
Primary language German
Location 🇪🇺 Heidelberg, Germany, Europe
CreditAttribution: sanduhrs at erdfisch commented
Status: Needs work » Needs review
FileSize
2605290_12.patch16.22 KB

You are right of course – attached.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented
Status: Needs review » Reviewed & tested by the community

phpcs says the only remaining problems are global names, type hinting, and 'Use function ip_address() instead of $_SERVER['REMOTE_ADDR'].'

We can't fix those without changing code.

Marking RTBC again.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 as a volunteer commented

See also: #2624926: Refactor run-tests.sh for Console component.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

Committed b13967f and pushed to 8.0.x and 8.1.x. Thanks!

  • alexpott committed cc75430 on 8.1.x 
    Issue #2605290 by sanduhrs, Mile23: Improve docs, coding standards for...

  • alexpott committed b13967f on 
    Issue #2605290 by sanduhrs, Mile23: Improve docs, coding standards for...

Status: Fixed » Closed (fixed)

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