Warning about an invalid amount of parameters to a method call is not apparent

[JoelLarson]

This test case demonstrates that a PHP call will silently ignore the E_WARNING that is thrown by PHP due to PhpClass->add() not having enough arguments to be satisfied. This has lead to some unintentional side effects while testing.

<?php

class PhpClass
{
    public function add($name, $email, $password, $knownPerson = false)
    {
        return [
            'name' => $name,
            'email' => $email,
            'password' => $password,
            'knownPerson' => $knownPerson
        ];
    }
}

describe('PhpClass', function () {
    context('when calling a method with too few parameters', function () {
        it('should bail and give an error', function () {

            $phpClass = new PhpClass();

            $output = $phpClass->add();

            expect($output)->to->equal([
                'name' => null,
                'email' => null,
                'password' => null,
                'knownPerson' => false
            ]);

        });
    });
});

Output:

vagrant@localhost:/var/www/project$ ./vendor/bin/peridot tests/spec/Error.spec.php 

  PhpClass
    when calling a method with too few parameters
      ✓ should bail and give an error

  1 passing (9 ms)

The test error was picked up and accepted once I added an event handler on the Emitter "error" event. It might be worth adding some kind of functionality that has a basic error_handling event attached that can be overwritten easily?

[brianium]

Certainly related to #157. This kind of thing makes me lean towards a peridot-strict plugin, or a --strict setting. Or as you suggest above, maybe the Runner could expose a strict setting that can be easily enabled via config?

[JoelLarson]

I'll just reference both here since this issue/ticket is more important to me.

The biggest problem here is that I've had quite a few tests break after applying my emitter event hook. Problems are undefined constants being ignored, invalid amounts of arguments in code being ignored and given NULL, having user errors not being populated.

Practically everything in PHP that does not throw an exception but is an notice/warning/error ends with the test passing. I will agree that notices are normally fine to ignore, but there are a lot of scenarios where warnings (like invalid amounts of arguments) are very valuable.

• Documentation doesn't really solve this problem very well, so I'll skip that one as an option.
• Adding this functionality seems like a good idea to me. It would probably be implemented where there is an emitter priority system where you can override it would be nice.
• Notices, warnings, and errors triggered by users are quite subjective; however, these things emitted by PHP I don't believe are quite as subjective. Having a way to differentiate them might be worth investigating.
• peridot-strict sounds like it would be a great plugin. The implementation feels more like it should be part of core though and not an additional dependency being hinted at by composer.

[JoelLarson]

I think it would be fine to just provide a very basic amount of detection for types of errors that can be easily overwritten. Then maybe provide a peridot-strict to add additional advanced functionality to really buckle down on possible errors that are preventable.

[brianium]

I see your point. I agree that a plugin would be a little annoying for something so essential. I don't believe Evenement has a way to control priority short of removing listeners - which might be fine? I might need a little help answering a couple of questions:

• What should default functionality be? Is the current functionality ok as a default?
• How should the core API change to handle this? Currently the Runner class controls how errors are handled during a run: https://github.com/peridot-php/peridot/blob/master/src/Runner/Runner.php#L48

Events are a great way to handle configuration, I am just unsure of what makes sense as a default case, and what part of the API should be responsible for managing things like error levels and differentiating between them. Does it make sense to take a cue from PHPUnit on the matter?

[brianium]

I had a feature slated for 2.0 mentioned in #132 - a little related in that it would give the feedback, but would not fail the test.