Scenario Provisioning: Thoughts on new feature?

[ghost]

So I've been out of the PHP game for a few months and as a 'back-on-the-horse' exercise, I wanted to play around with Peridot as I really like the syntax (coming from a PHPUnit guy). While I was tinkering, I thought it'd be nice to have a feature wherein specific 'scenarios' are bound to a test, allowing the same test definition to run with different values (similar to data providers in PHPUnit). I wanted to take a stab at implementing this, so I forked the repo and implemented scenarios as a kind of proof of concept and was wondering what the Peridot powers that be think of the feature in general? Consider a FizzBuzz example:

function convertValueToFizzBuzz($value)
{
    $response = '';

    if ($value % 3 === 0) {
        $response .= 'Fizz';
    }

    if ($value % 5 === 0) {
        $response .= 'Buzz';
    }

    return ($response ?: $value);
}

A simple test for 'Fizz' conversion could be described as:

describe('convertValueToFizzBuzz($value)', function() {
    context('when $value divisible by 3', function() {
        it('should return "Fizz"', function() {
            assert(convertValueToFizzBuzz(9) === 'Fizz');
        });
    });
});

However, it would be nice to test other values as well. This could be accomplished by looping over a value array within the test, but this just adds noise to the test itself. What I had in mind was adding a function to the base DSL that would attach a scenario to the last-added test (the test would run once for each scenario):

describe('convertValueToFizzBuzz($value)', function() {
    context('when $value divisible by 3', function() {
       it('should return "Fizz"', function() {
           assert(convertValueToFizzBuzz($this->scenario['expected_fizz_value']) === 'Fizz');
       });
       inScenario(['expected_fizz_value' => -3]);
       inScenario(['expected_fizz_value' => 3]);
       inScenario(['expected_fizz_value' => 6]);
    });
});

The idea is that a scenario is an object which is basically a glorified array. Each scenario in turn is injected into the scope right after a given test's setup functions as <test scope>->scenario. The scenario is removed from the test scope immediately before the test teardown functions are invoked.

Because each test would be run multiple times, multiple events would be emitted per-test. This leads to some duplicate reporting, but the duplication makes it very easy to see which scenario is causing the test to fail:

$ bin/peridot fixtures/sample-scenario-spec.php 

  convertValueToFizzBuzz($value)
    when $value divisible by 3
      ✓ should return "Fizz"
      ✓ should return "Fizz"
      ✓ should return "Fizz"


  3 passing (0 ms)

<...changes made causing a scenario to fail...>

$ bin/peridot fixtures/sample-scenario-spec.php 

  convertValueToFizzBuzz($value)
    when $value divisible by 3
      ✓ should return "Fizz"
      1) should return "Fizz"
      ✓ should return "Fizz"


  2 passing (0 ms)
  1 failing

I know that this is a trivial example but hopefully it serves to illustrate the idea. Any thoughts?

Josh

[brianium]

I think this is actually really cool and useful! I am open to the idea, but the first question I would ask: Can this be implemented via a plugin instead of merged into core? Peridot is pretty extensible, so if at all possible, a plugin would be preferred

[ghost]

I haven't played around with the plugin/event system much, but I suspect it could be. I'll take a look and report back.

[ghost]

Alrighty, I set up a new repo with a very rough proof-of-concept plugin pushed to dev branch. Appears to be working with only a 1-line change to core...in Peridot\Core\Context, emit a new test.added event when a test is added to a suite:

/**
 * Create a test and add it to the current suite
 *
 * @param $description
 * @param $fn
 */
public function addTest($description, callable $fn = null, $pending = null)
{
    $test = new Test($description, $fn);
    if ($pending !== null) {
        $test->setPending($pending);
    }
    $test->setFile($this->file);
    $this->getCurrentSuite()->addTest($test);
    $this->eventEmitter->emit('test.added', $test); // <-- new event

    return $test;
}

Slightly altered the scenario syntax:

describe('convertValueToFizzBuzz($value)', function() {

    context('when $value is divisible by 3 and 5', function() {
        it('should return "FizzBuzz"', function() {
            assert(convertValueToFizzBuzz($this->expected_fizzbuzz_value) === 'FizzBuzz');
        });
        inScenario( setUp(function() { $this->expected_fizzbuzz_value = 0; }) );
        inScenario( setUp(function() { $this->expected_fizzbuzz_value = 15; }) );
        inScenario( setUp(function() { $this->expected_fizzbuzz_value = 45; }) );
        inScenario( setUp(function() { $this->expected_fizzbuzz_value = 60; }) );
    });

});

This syntax allows for a more powerful scenario setup/teardown system and makes the plugin code which binds the scenarios to the test definition more readable and easy to follow (I think). Repo includes a peridot.php file with plugin setup. The main functionality can be broken down as:

• Autoload an additional DSL file which defines the inScenario() function (as well as a few helpers)
• Define a Peridot\Plugin\Scenarios\Plugin singleton which manages plugin context
• Keep track of the last-added test within the plugin context via the new test.added event emitted from Peridot\Core\Context
• Calling inScenario() DSL function creates a new Scenario object, which is associated with the last-added test within the plugin context
• When a test finally runs, plugin context responds to the test.start event before the test runs and hooks in the scenarios associated with the test.

The method of hooking in the scenarios is a bit sketch but it seems to work. Essentially, a test execution follows the path in the core code:

Setup test suite context (all **`beforeEach`** definitions)
Run test definition
Tear down test suite context (all **`afterEach`** definitions)

We can break into this path by adding an additional setup and teardown function before the test runs:
Inside the new setup function, we actually execute the test definition against each test scenario except the last. This is done by looping over the scenarios and essentially using their setup/teardown methods just like a standard suite setup/teardown is used when running the test definition. We want to execute all but the last scenario because the core code is always going to call the test definition after all setup code completes, so there has to be an active scenario in-scope during that execution of the definition. To ensure this, the last thing the new setup function does is run the setup for the last scenario.
Inside the new teardown function, we simply run the teardown function of the last scenario.
This means that the new logical execution steps are:

Setup test suite context (all **`beforeEach`** definitions)
Execute (N-1) scenarios against test definition (scen. setup / test def / scen. teardown)
Scenario N setup
Run test definition
Scenario N teardown
Tear down test suite context (all **`afterEach`** definitions)

Hopefully that makes some sense. Like I said, the code in the repo is very rough but if this direction makes sense, I'll go through and give it a thorough polishing (comments, tests, etc). Let me know what you think!

[brianium]

I think this approach sounds fine. Sounds like an overall small footprint on core which I like :)

From a plugin usability perspective, it seems like there needs to be care taken to make sure inScenario is called after it, other wise you might end up with some unexpected behavior. Maybe there are some constraints that can be placed on the call to inScenario, like is the last added Test a Suite?. Maybe the order of operations is ok as long as it is documented within the plugin.

I would also make sure failures in a scenario's setUp or tearDown report useful information via the reporter.

I am happy to accept a PR to core to support the new event. I'll update docs once it's added, or I can hold off until your plugin is fleshed out in case you have some more events you want to add.

In general, seems like a super useful addition.

Thanks!

[ghost]

Re-read my last comment and realized how long it was, hopefully it didn't bore you too much ;) Definitely trying to minimize core impact! Yeah, I've considered how to ensure scenarios are bound directly to tests, have some ideas. I'll hold off on the PR until the plugin is a bit more fleshed out and polished.

Thanks!
Josh

[brianium]

No worries man! You are putting in unpaid time to make cool things with computers. I can at least give up a little time too discuss :)

Thanks again!