Problem/Motivation

When using the simpletest ui to run tests, pressing the "Clean Environment" button while a test is selected will run the selected tests instead of cleaning the environment.

Steps to reproduce:

  1. Standard Drupal 8 Install
  2. Enable Simpletest module (Testing)
  3. Visit simpletest ui (admin/config/development/testing)
  4. Select any test or set of tests
    Click "Clean Environment"
  5. Note that test(s) start running

This could be changed to minor since the clean environment button is functional without a test selected and the testing interface is not part of normal site development.

Proposed resolution

"Clean Environment" button should be properly identified as triggered element

Remaining tasks

User interface changes

none

API changes

none

Data model changes

none

CommentFileSizeAuthor
#3 2554267-3.patch1.75 KBasgorobets
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

justAChris created an issue. See original summary.

othermachines’s picture

Confirmed (using latest 8.0.x). At first I thought I was imagining things.

asgorobets’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
1.75 KB

The issues was a bit tricky to find, but finally got it working.

First of all, there is already a separate submit handler for 'Clean environment' button, it's just not working properly under some circumstances.
Submit handlers attached to $form['clean']['op'] buttons are not executed when ticking at least one checkbox, but they are working fine when no checkboxes are ticked. This happens because Drupal does not identify correctly the triggered element. When ticking at least one checkbox, triggered element is considered the 'Run tests' button as it can't find the element (button) that was clicked in the form, and it has to default to the first button.

Button is missing in the $form because there is an early return happening when constructing $form array, if at least one of the tests is checked, then it will not construct the form again on submit, with this it will lose any evidence of 'clean' fieldset and 'op' button, that is added after this return statement.

    // Do not needlessly re-execute a full test discovery if the user input
    // already contains an explicit list of test classes to run.
    $user_input = $form_state->getUserInput();
    if (!empty($user_input['tests'])) {
      return $form;
    }
    ...

    $form['clean'] = array(
      '#type' => 'fieldset',
      '#title' => $this->t('Clean test environment'),
      '#description' => $this->t('Remove tables with the prefix "simpletest" and temporary directories that are left over from tests that crashed. This is intended for developers when creating tests.'),
      '#weight' => 200,
    );
    $form['clean']['op'] = array(
      '#imput' => TRUE,
      '#type' => 'submit',
      '#value' => $this->t('Clean environment'),
      '#submit' => array('simpletest_clean_environment'),
    );

In order to mitigate the issue I propose to move the 'clean' fieldset up and place it after the 'actions' element, so it is always rendered, even if the early return happens.

Please see patch attached.

LoMo’s picture

Status: Needs review » Reviewed & tested by the community

I marked this is RTBC and made a comment yesterday, but there must have been a glitch since it's gone now.

I did test this, even after running some tests so that there really was an environment to 'clean' (that took a while, since I guess the tests I selected were not the simplest ones). And I verified the problem and that the solution seems to work, as advertised. I think this can be marked RTBC. Nice work, asgorobets. :-)

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Permitted in beta since this is test related code and fixing a bug. Committed 983d2eb and pushed to 8.0.x. Thanks!

  • alexpott committed 983d2eb on 8.0.x
    Issue #2554267 by asgorobets, justAChris, LoMo: Clean Environment button...

Status: Fixed » Closed (fixed)

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