Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Test] Guidelines proposal to test the commands interactive mode. #2972

Merged
merged 11 commits into from
Dec 30, 2016

Conversation

MontealegreLuis
Copy link
Member

The goal of this PR is to set the path to a more uniform way to test commands using the interactive mode. I'm using the AuthenticationProviderCommand as an example. There should be some similarities for all the commands that generate code.

First, I replaced most of the traits from the command with classes and its corresponding tests. I identified 2 main collaborator objects.

  • ConfirmGeneration which is the object that stops the command in case the user cancels the operation and
  • AuthenticationProviderQuestions which is the object that asks the user for any required parameter that might be missing, in this command is the module, the class and the provider-id.

Both objects require the use of the DrupalStyle class, which is the first big change to the code, because it requires to add the $input and $ouput objects as a services. Please take a look to bin/drupal.php, config/services/drupal-console/generate.yml and services.yml to see how this affects the configuration.

The AuthenticationProviderQuestions class needs to use the extensions Manager, which is hard to mock because of its chained method calls.

$manager
        ->discoverModules()
        ->showInstalled()
        ->showUninstalled()
        ->showNoCore()
        ->getList(true)
;

I replaced this sequence of calls with 2 new methods Manager#showModuleNamesExceptCore and Manager#showAllProfileNames. In order to replace the manager with a double in an easier way. I'm open to suggestions for better names for this 2 methods.

Having said that, I'll explain the organization of the tests. I did several opinionated changes to the tests structure which I'd like to discuss/explain as needed.

  • I'm using the @test annotation instead of using the prefix test for method names.
  • I'm using snake_case and the format it_behaves_this_way similar to what you do with phpspec
  • All the instance variables are at the bottom of the file
  • I'm using the @before annotation instead of a setUp method. I named this method configure as a convention and its structure is similar to this one in AuthenticationProviderQuestionsTest.
/** @before */
public function configure()
{
    $this->configureCollaborators(); // Setup for test doubles
    $this->initValues(); // Set the expected values and parameters
    $this->createSUT(); // Create the Subject Under Test
}

The method initValues is optional as you can see in AuthenticationProviderCommandTest where the initialization of the values is in the field declaration. I'm asking for your input on these 2 options. Which one would you like? I prefer the initialization on the field declaration because you can go directly to the field declaration and see what the value is, without having to go to the method declaration.

  • All the tests methods are at the top of the file followed by the @before method. This order makes test methods easy to find. You don't have to navigate to them because it's the first thing to find on any test class. For instance:
/** @test */
function it_confirms_positively_when_default_value_is_set_to_true() {}

/** @test */
function it_confirms_when_user_answer_is_positive() {}

/** @test */
function it_shows_a_warning_if_user_cancels_generation() {}

/** @before */
public function configure() {}
  • The specific configuration of doubles is behind methods that explain how the double affects the object under test. For instance:
private function userConfirmsGeneration()
{
    $confirm = true;
    $this->style->confirm(Argument::any(), true)->willReturn($confirm);
}
  • Having those methods using that naming schema, we can have test methods organized like this:
// Given
$this->userConfirmsGeneration();

// When
$code = $this->tester->execute(
    $this->withAllOptions(),
    $this->nonInteractive
);

// Then
$this
    ->generator
    ->generate($this->module, $this->class, $this->providerId)
    ->shouldHaveBeenCalled()
;
$this->assertEquals(0, $code);

I know this changes are costly in time if you try to the same for all the commands, but I think its worth it. I'm willing to pair with you or review the tests for new code. Or also do some pairing or help documenting the process to add tests to existing code.

Thank you all.

@MontealegreLuis MontealegreLuis changed the title Develop Guidelines proposal to test the commands interactive mode. Nov 21, 2016
@jmolivas jmolivas changed the title Guidelines proposal to test the commands interactive mode. [Test] Guidelines proposal to test the commands interactive mode. Dec 17, 2016
@jmolivas jmolivas merged commit 03c8176 into hechoendrupal:develop Dec 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants