Part of #1696224: Remove system_settings_form()
simpletest_settings_form uses system_settings_form() and needs to use system_config_form().

Files: 
CommentFileSizeAuthor
#60 drupal8.config-simpletest.60.patch358 bytesdagmar
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.config-simpletest.60.patch. Unable to apply patch. See the log in the details link for more information. View
#56 drupal8.config-simpletest.56.patch12.66 KBsun
PASSED: [[SimpleTest]]: [MySQL] 41,105 pass(es). View
#45 1705748-simpletest-config.patch13 KBboombatower
PASSED: [[SimpleTest]]: [MySQL] 40,778 pass(es). View
#39 drupal8.config-simpletest.39.patch14.5 KBsun
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View
#38 interdiff.txt700 bytestim.plunkett
#37 simpletest-1705748-37-fail.patch12.06 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View
#37 simpletest-1705748-37-pass.patch12.06 KBtim.plunkett
PASSED: [[SimpleTest]]: [MySQL] 39,900 pass(es). View
#31 simpletest-1705748-31.patch12.06 KBno_commit_credit
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: Tests were executed, but no results were found.. View
#31 interdiff.txt506 bytesno_commit_credit
#28 config-simpletest.28.patch12.06 KBn3or
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View
#28 interdiff.txt5.09 KBn3or
#24 config-simpletest.24.patch12.4 KBn3or
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch config-simpletest.24.patch. Unable to apply patch. See the log in the details link for more information. View
#24 interdiff.txt655 bytesn3or
#22 config-simpletest.20.patch12.36 KBn3or
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View
#22 interdiff.txt1.38 KBn3or
#18 config-simpletest.18.patch12.09 KBn3or
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View
#18 interdiff.txt2.51 KBn3or
#13 config-simpletest.12.patch11.85 KBn3or
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View
#13 interdiff.txt1.66 KBn3or
#12 config-simpletest.12.patch11.85 KBn3or
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View
#12 interdiff.txt1.66 KBn3or
#9 config-simpletest.9.patch11.54 KBn3or
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View
#9 interdiff.txt449 bytesn3or
#8 config-simpletest.7.patch11.53 KBn3or
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View
#8 interdiff.txt2.06 KBn3or
#5 config-simpletest.5.patch11.47 KBn3or
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View
#5 interdiff.txt4.73 KBn3or
#3 config-simpletest.3.patch12.07 KBn3or
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View

Comments

sun’s picture

Tagging.

n3or’s picture

Assigned: Unassigned » n3or

Assignment.

n3or’s picture

Status: Active » Needs review
FileSize
12.07 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View

First version. Tried to adjust naming to the conventions, converted the admin form, created the yml-file and the upgrade path.

sun’s picture

Status: Needs review » Needs work

Awesome! :)

+++ b/core/modules/simpletest/config/simpletest.settings.yml
@@ -0,0 +1,7 @@
+clear_results: 1

In YAML, all values are strings by default, so strings do not need any special treatment.

However, integers, numbers and any other special values need to be wrapped in single-quotes ('').

+++ b/core/modules/simpletest/config/simpletest.settings.yml
@@ -0,0 +1,7 @@
+httpauth.password:
+httpauth.username:

Likewise, empty strings need to be explicitly specified as such; i.e.: ''

+++ b/core/modules/simpletest/config/simpletest.settings.yml
@@ -0,0 +1,7 @@
+parent_profile: 0

This value should be an empty string.

+++ b/core/modules/simpletest/simpletest.install
@@ -171,12 +171,25 @@ function simpletest_uninstall() {
   // Remove settings variables.
-  variable_del('simpletest_httpauth_method');
-  variable_del('simpletest_httpauth_username');
-  variable_del('simpletest_httpauth_password');
-  variable_del('simpletest_clear_results');
-  variable_del('simpletest_verbose');
+  config('simpletest.settings')
+    ->delete('clear_results')
+    ->delete('httpauth.method')
+    ->delete('httpauth.password')
+    ->delete('httpauth.username')
+    ->delete('maximum_redirects')
+    ->delete('parent_profile')
+    ->delete('verbose');

All configuration objects of a module are automatically deleted upon uninstallation. These lines need to be removed entirely instead. :)

+++ b/core/modules/simpletest/simpletest.install
@@ -171,12 +171,25 @@ function simpletest_uninstall() {
+function simpletest_update_8000() {
+   update_variables_to_config('simpletest.settings', array('simpletest_clear_results' => 'clear_results'));
+   update_variables_to_config('simpletest.settings', array('simpletest_httpauth_password' => 'httpauth.method'));
+   update_variables_to_config('simpletest.settings', array('simpletest_httpauth_password' => 'httpauth.password'));
+   update_variables_to_config('simpletest.settings', array('simpletest_httpauth_username' => 'httpauth.username'));
+   update_variables_to_config('simpletest.settings', array('simpletest_maximum_redirects' => 'maximum_redirects'));
+   update_variables_to_config('simpletest.settings', array('simpletest_parent_profile' => 'parent_profile'));
+   update_variables_to_config('simpletest.settings', array('simpletest_verbose' => 'verbose'));
+}

ugh :) Please check the patch in http://drupal.org/node/1496458#comment-6285700 for how this should look like ;)

+++ b/core/modules/simpletest/simpletest.pages.inc
@@ -436,6 +436,7 @@ function simpletest_result_status_image($status) {
+  $simpletest_config = config('simpletest.settings');

The variable can be renamed to just $config, since this is the settings form of Simpletest module, so there's naturally no other $config involved.

Lastly, it's possible that we might miss some instances in core/scripts/run-tests.sh -- or did you check that already?

n3or’s picture

FileSize
4.73 KB
11.47 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View

Fixed problems mentioned in comment #4. Thank you!

n3or’s picture

Status: Needs work » Needs review
sun’s picture

Status: Needs review » Needs work
+++ b/core/modules/simpletest/config/simpletest.settings.yml
@@ -1,7 +1,7 @@
+httpauth.method: '1'
+httpauth.password: ''
+httpauth.username: ''

oh, sorry, I completely overlooked a blatant mistake here -- every dot in a key is transformed into a nested array key in the .yml file. I.e., this needs to look like this:

httpauth:
    method: '1'
    password: ''
    username: ''
+++ b/core/modules/simpletest/lib/Drupal/simpletest/TestBase.php
@@ -483,10 +483,11 @@ abstract class TestBase {
-    $this->httpauth_method = variable_get('simpletest_httpauth_method', CURLAUTH_BASIC);
...
+    $this->httpauth_method = $simpletest_config->get('httpauth.method');

We most likely need to cast the value of httpauth.method into an integer like this:

$this->httpauth_method = (int) $config->get(...);

That is, because the value 1 refers to the cURL constant value CURLAUTH_BASIC, and cURL won't know what to do with a '1' (string).

+++ b/core/modules/simpletest/simpletest.install
@@ -170,13 +170,18 @@ function simpletest_uninstall() {
+function simpletest_update_8000() {

We're still missing the phpDoc for this function :)

+++ b/core/modules/simpletest/simpletest.install
@@ -170,13 +170,18 @@ function simpletest_uninstall() {
+  update_variables_to_config('system.maintenance', array(

The variables are migrated into the wrong config object! ;)

+++ b/core/modules/simpletest/simpletest.install
@@ -170,13 +170,18 @@ function simpletest_uninstall() {
+    'simpletest_httpauth_password' => 'httpauth.method',

Variable mismatch! :)

n3or’s picture

Status: Needs work » Needs review
FileSize
2.06 KB
11.53 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View

Problems mentioned in comment #7 fixed.

n3or’s picture

FileSize
449 bytes
11.54 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View

4 spaces instead of 2 spaces in yml-file.

Status: Needs review » Needs work

The last submitted patch, config-simpletest.9.patch, failed testing.

sun’s picture

+++ b/core/modules/simpletest/lib/Drupal/simpletest/Tests/SimpleTestTest.php
@@ -76,7 +76,7 @@ class SimpleTestTest extends WebTestBase {
-      variable_set('simpletest_maximum_redirects', 1);
+      config('simpletest.settings')->set('maximum_redirects', 1);

+++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
@@ -625,7 +625,7 @@ abstract class WebTestBase extends TestBase {
-    variable_set('simpletest_parent_profile', $this->originalProfile);
+    config('simpletest.settings')->set('parent_profile', $this->originalProfile);

We're missing a ->save() here. :)

n3or’s picture

FileSize
1.66 KB
11.85 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View

Problem mentioned in #11 fixed and changed a comment related to simpletest_parent_profile.

n3or’s picture

Status: Needs work » Needs review
FileSize
1.66 KB
11.85 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: PHP Fatal error encountered during run_tests.sh. See review log for details.. View

Forgot to change issue status.

Status: Needs review » Needs work

The last submitted patch, config-simpletest.12.patch, failed testing.

sun’s picture

I don't see what's wrong with this patch.

Most likely, the test failures are caused by the change to drupal_system_listing(), which in turn means that also this patch is blocked on #1704196: Remove Config's dependencies on procedural Drupal code in includes/common.inc - which will hopefully land very soon.

aspilicious’s picture

Status: Needs work » Needs review
Issue tags: -API change, -Configuration system, -Config novice

#13: config-simpletest.12.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +API change, +Configuration system, +Config novice

The last submitted patch, config-simpletest.12.patch, failed testing.

n3or’s picture

Status: Needs work » Needs review
FileSize
2.51 KB
12.09 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View

Problem fixed by using "maximum_redirects" as property of TestBase and not of config object. Thanks to @sun for his support!

Additionally added missing comma in simpletest.install.

sun’s picture

Status: Needs review » Needs work
+++ b/core/modules/simpletest/lib/Drupal/simpletest/TestBase.php
@@ -483,13 +483,17 @@ abstract class TestBase {
+    $simpletest_config = config('simpletest.settings');
...
+    $this->maximumRedirects = config('simpletest.settings')->get('maximum_redirects');

We already have that config object in $simpletest_config here? :)

alexpott’s picture

Status: Needs work » Needs review

Couple of minor nitpicks...

+++ b/core/modules/simpletest/simpletest.pages.incundefined
@@ -436,6 +436,7 @@ function simpletest_result_status_image($status) {
  * @see simpletest_settings_form_validate()
  */
 function simpletest_settings_form($form, &$form_state) {
+  $config = config('simpletest.settings');

Need to add @see simpletest_settings_form_submit()

+++ b/core/modules/simpletest/simpletest.pages.incundefined
@@ -491,7 +492,22 @@ function simpletest_settings_form($form, &$form_state) {
+/**
+ * Form submission handler for simpletest_settings_form().
+ *
+ * @ingroup forms
+ */
+function simpletest_settings_form_submit($form, &$form_state) {

The @ingroup forms here is unnecessary. It's for functions that produce form arrays not submission handlers

alexpott’s picture

Status: Needs review » Needs work

Crosspost...

n3or’s picture

Status: Needs work » Needs review
FileSize
1.38 KB
12.36 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View

You're absolutely right!

n3or’s picture

Status: Needs review » Needs work

Missing changes of #21!

n3or’s picture

Status: Needs work » Needs review
FileSize
655 bytes
12.4 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch config-simpletest.24.patch. Unable to apply patch. See the log in the details link for more information. View

Fixed problems mentioned in #20

sun’s picture

Status: Needs review » Postponed

Yay! :)

This patch is RTBC...

...however, the additionally required fix for maximumRedirects also applies to the verbose setting, which makes this super-nice patch blocked by #1611430: Verbose debug output in unit tests relies on the database

That is, because the config() in simpletest_verbose() will effectively try to read the verbose setting from the simpletest.settings config file of the "site under test" --- but that environment might not even have Simpletest module installed.

Essentially, this requires the very same change as for maximumRedirects - but that ain't possible, as long as TestBase calls into a completely detached procedural simpletest_verbose() function, which is not able to access the test class' properties anymore.

aspilicious’s picture

Status: Postponed » Needs review
Issue tags: -API change, -Configuration system, -Config novice

#24: config-simpletest.24.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +API change, +Configuration system, +Config novice

The last submitted patch, config-simpletest.24.patch, failed testing.

n3or’s picture

Status: Needs work » Needs review
FileSize
5.09 KB
12.06 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View

Fixed incompatibility to current D8 core. In my opinion attached interdiff doesn't make sense, I attached it though.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Awesome!!! :)

n3or’s picture

Component: configuration system » simpletest.module

Wrong component.

no_commit_credit’s picture

FileSize
506 bytes
12.06 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: Tests were executed, but no results were found.. View

Minor style fix attached. Per http://drupal.org/node/1354#hookimpl, update hooks get the imperative (the summaries are user-facing).

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 8.x. Thanks.

xjm’s picture

Title: Convert simpletest settings to configuration system » [TESTBOT BROKEN] Convert simpletest settings to configuration system
Category: task » bug
Priority: Normal » Critical
Status: Fixed » Active

This issue causes #1717868: PIFR uses variable_get() but needs config() in Drupal 8 (note how the patches above have 0 passes). I left a message asking webchick to git revert 490386f and reopen this issue if a solution for that other issue is not straightforward, because no tests are running for any projects since this was committed.

If that issue is resolved, we can set the status of this one back without reverting.

webchick’s picture

Category: bug » task
Priority: Critical » Normal
Status: Active » Needs work

Reverted this patch for now using the command in #34. Reverting status.

webchick’s picture

Title: [TESTBOT BROKEN] Convert simpletest settings to configuration system » Convert simpletest settings to configuration system

Almost.

tim.plunkett’s picture

Status: Needs work » Needs review
FileSize
12.06 KB
PASSED: [[SimpleTest]]: [MySQL] 39,900 pass(es). View
12.06 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View

On a hunch, let's see what these results are.

tim.plunkett’s picture

FileSize
700 bytes

Forgot an interdiff, here's a cheap one.
The results prove my theory, that these simpletest variables are responsible for the results persisting long enough for testbot to display results.

sun’s picture

FileSize
14.5 KB
PASSED: [[SimpleTest]]: [MySQL] 0 pass(es). View

Attached patch requires PIFR to invoke simpletest with --keep-results

tim.plunkett’s picture

Status: Needs review » Postponed

Split this out into #1719530: Add --keep-results flag to run-tests.sh so it can be committed without breaking core.

boombatower’s picture

Status: Postponed » Needs review

#31: simpletest-1705748-31.patch queued for re-testing.

boombatower’s picture

Testing out to see if a patched testbot will detect this fun edge-case since humans seem to miss the 0.

http://drupal.org/node/873496#comment-6332732

boombatower’s picture

Check out #31 vs #28! Fix http://drupal.org/node/873496#comment-6332868. Looking for feedback on the repercussions.

boombatower’s picture

Status: Needs review » Postponed
boombatower’s picture

Status: Postponed » Needs review
FileSize
13 KB
PASSED: [[SimpleTest]]: [MySQL] 40,778 pass(es). View

Now that #1719530: Add --keep-results flag to run-tests.sh made it in, I have patch a pifr bot in advance, and rerolled this patch.

boombatower’s picture

Note that I didn't have the pifr patch to catch the 0 results applied...only the patch to add flag when calling run-tests.sh...so it seems either run-tests.sh patch does not work of pifr patch does not.

boombatower’s picture

#45: 1705748-simpletest-config.patch queued for re-testing.

EDIT: flag patch fix was committed.
EDIT: Looks like someone broke my pifr code that allows me to disable a bot running a test so it won't ask for another...i'll have to retest again.
EDIT: tested on patch client and you will see my patch returned proper results.

EDIT: Hold off on committing until we can push to all testbots. #1723044: I no longer have access to update testbots

n3or’s picture

#45: 1705748-simpletest-config.patch queued for re-testing.

sun’s picture

boombatower’s picture

It ran run-tests.sh with new flag properly, from log:

/usr/bin/php ./core/scripts/run-tests.sh --concurrency 8 --php /usr/bin/php --url 'http://drupaltestbot659-mysql/checkout' --keep-results --all

So looks like we should be able to commit this safely now. Along with that deployment is the one to catch crud like this in the future so we hopefully can stop having these sorts of rare issues.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Thanks.

boombatower’s picture

Actually, before we jump too fast. Based on your comment http://drupal.org/node/1717868#comment-6422150 I looked into the --verbose flag which I remember existing, but not to replace the simpletest_verbose variable.

  --verbose   Output detailed assertion messages in addition to summary.

Which is intended to spit out the individual row-by-row messages (assertions) in the output of the run-tests.sh script. Which is quite different from the simpletest_verbose variable which was to control the verbose page capturing functionality in DrupalWebTestCase.

We probably want to split these out and rename one. May also want to default to false on the command line? dunno.

I think --verbose flag as it stand now works the way that is most consistent with other scripts and makes sense. Perhaps we rename simpletest_verbose variable to 'capture' or something along those lines to 'capture browser shot' etc. I don't like those names just providing a thought basis.

If we default cli to false then using the cli then might do.

--print-details --verbose

--print-details to print out assertion messages
--print-results
--results
--details
,etc

sun’s picture

Status: Reviewed & tested by the community » Needs work

@boombatower: Let's move that discussion on --verbose into a separate issue. This patch here doesn't actually influence that, as it doesn't touch run-tests.sh and so on at all.

+++ b/core/modules/simpletest/config/simpletest.settings.yml
@@ -0,0 +1,8 @@
+maximum_redirects: '5'

+++ b/core/modules/simpletest/lib/Drupal/simpletest/TestBase.php
@@ -541,13 +543,16 @@ abstract class TestBase {
+    // Maximum redirects setting for the simpletest browser.
+    $this->maximumRedirects = $simpletest_config->get('maximum_redirects');

+++ b/core/modules/simpletest/lib/Drupal/simpletest/Tests/SimpleTestTest.php
@@ -84,7 +84,7 @@ class SimpleTestTest extends WebTestBase {
+      $this->maximumRedirects = 1;

+++ b/core/modules/simpletest/lib/Drupal/simpletest/WebTestBase.php
@@ -911,7 +911,7 @@ abstract class WebTestBase extends TestBase {
     // TODO: Remove this for Drupal 8, since fixed in curl 7.20.0.
...
+    if (in_array($status, array(300, 301, 302, 303, 305, 307)) && $this->redirect_count < $this->maximumRedirects) {

+++ b/core/modules/simpletest/simpletest.install
@@ -170,13 +170,21 @@ function simpletest_uninstall() {
+    'simpletest_maximum_redirects' => 'maximum_redirects',

I wonder whether maximum redirects is actually supposed to be configuration...?

For one, there's at least one test that overrides it, so it appears to be test-case-specific setting/property.

The only other usage is within WebTestBase's internal browser.

In turn, this means that you cannot actually configure this... if you'd configure it to e.g. 0, then surely some tests will fail, because the internal browser would not follow redirects anymore.

Even more interesting is the @todo about it being potentially obsolete for D8, since cURL itself was fixed.

So I guess we want to remove this from the configuration and just turn it into a regular class property.

+++ b/core/modules/simpletest/config/simpletest.settings.yml
@@ -0,0 +1,8 @@
+parent_profile: ''

+++ b/core/modules/simpletest/simpletest.install
@@ -170,13 +170,21 @@ function simpletest_uninstall() {
+function simpletest_update_8000() {
...
+    'simpletest_parent_profile' => 'parent_profile',

This variable actually does not exist on a regular Drupal site, but only within a test run.

Let's delete it from the default config file and also from the update function.

sun’s picture

Additionally, let's make sure to remove the stop-gap fix to run-tests.sh that's going to be added in #1719530-15: Add --keep-results flag to run-tests.sh

boombatower’s picture

Status: Needs work » Reviewed & tested by the community

When I referred to flag it was from your comment in http://drupal.org/node/1717868#comment-6422150, which would seem to imply the testbot should be using the --verbose flag, yet then you had "(non-existing)" which I am not sure if that referred to not adding one for the old setting or what since we do have a flag it just doesn't related to what is being changed.

btw, the testbot is not properly picking up the (non-existing) --verbose flag.

Since we have the temporary hack from the other issue placing the --verbose discussion in another issue is fine, but without that it definitely needed to be solved in this issue along with the --keep-results shift.

Created #1774002: Correct mistake introduced in simpletest cmi conversion by introducing --verbose-browser flag

sun’s picture

FileSize
12.66 KB
PASSED: [[SimpleTest]]: [MySQL] 41,105 pass(es). View

Adjusted for #53 and also removed the stop-gap fix from run-tests.sh, which is no longer needed with this patch.

boombatower’s picture

Why remove the stop gap measure now since it will simple return to being enabled on the testbot until we resolve and commit #1774002: Correct mistake introduced in simpletest cmi conversion by introducing --verbose-browser flag which seemed to be your preference to split into another issue?

sun’s picture

@boombatower: The testbot already adheres to the converted configuration, as you can see in the test results for the patch in #56: http://qa.drupal.org/pifr/test/332313

The separate --verbose-browser option (as proposed in #1774002: Correct mistake introduced in simpletest cmi conversion by introducing --verbose-browser flag) is not relevant for this conversion issue.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Right; the workaround was for these settings not being converted to CMI yet; I don't think it has bearing on #1774002: Correct mistake introduced in simpletest cmi conversion by introducing --verbose-browser flag.

Ok, here we go. Second time's a charm? :)

Committed and pushed to 8.x. Thanks!

dagmar’s picture

Status: Fixed » Needs review
FileSize
358 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal8.config-simpletest.60.patch. Unable to apply patch. See the log in the details link for more information. View

I think the yml file was not committed.

webchick’s picture

Status: Needs review » Fixed

*blush*

Committed/pushed to 8.x. Thanks. :P

boombatower’s picture

so yea, I tested/committed/wrote the stuff to make the testbot "convert to cmi" it isn't convert it to cmi...we decided to change to flags instead of setting the configuration directly like testbot did before... unfortunately we did not add a flag to turn off verbose test output and the default remains enabled...so the testbot should now be testing with verbose browser output.

Which is why it seemed logical to either a) include the --verbrose-browser flag in this patch, or before this patch (like with --keep-results), or b) leave the temporary hack in. Whereas now we are just back to a broken implementation.

boombatower’s picture

The patch for adding --keep-results http://drupal.org/node/1719530#comment-6326150 also subtly change simpletest_verbose to be set from --verbose flag and failed to update the documentation. So given that I looked at the code before, documentation, and intentent this does not behave the way it was documented. We can resolve the documentation in the other issue for --verbose-browser if we choose to change or just fix doc.

So this issue is fine due to the changes made in --keep-results (doesn't just add keep results flag).

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