Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

Commit

Permalink
[#72] Remove tests specific to ArrayInput on InputFilter suite
Browse files Browse the repository at this point in the history 
ArrayInput specific logic branch its not longer present and current Input tests are enough
  • Loading branch information
@Maks3w
Maks3w committed Oct 8, 2015
1 parent a816b14 commit 3ffd32c
Showing 1 changed file with 0 additions and 26 deletions.
26 changes: 0 additions & 26 deletions test/BaseInputFilterTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use PHPUnit_Framework_MockObject_MockObject as MockObject;
use PHPUnit_Framework_TestCase as TestCase;
use stdClass;
use Zend\InputFilter\ArrayInput;
use Zend\InputFilter\BaseInputFilter;
use Zend\InputFilter\Exception\InvalidArgumentException;
use Zend\InputFilter\Exception\RuntimeException;
Expand Down Expand Up @@ -556,22 +555,6 @@ public function testAddingExistingInputWillMergeIntoExisting()
$this->assertFalse($filter->get('foo')->isRequired());
}

/**
* @group 5638
*/
public function testPopulateSupportsArrayInputEvenIfDataMissing()
{
/** @var ArrayInput|MockObject $arrayInput */
$arrayInput = $this->getMock(ArrayInput::class);
$arrayInput
->expects($this->once())
->method('resetValue');

$filter = $this->inputFilter;
$filter->add($arrayInput, 'arrayInput');
$filter->setData(['foo' => 'bar']);
}

/**
* @group 6431
*/
Expand All @@ -597,15 +580,6 @@ public function testMerge()
);
}

public function testHasValueIsFalseIfNoValueWasProvided()

This comment has been minimized.

Sign in to view

Copy link
@BreiteSeite

BreiteSeite Oct 8, 2015

Contributor

@Maks3w Why did you removed this test? It tests behavior, not code and should stay so regressions are noticed.

This comment has been minimized.

Sign in to view

Copy link
@Maks3w

Maks3w Oct 8, 2015

Author Member

For these reasons:

  1. BaseInputFilter its decoupled of implementations and assert resetValue is called is enough. The behavior of resetValue its tested by the specific input test suite.
  2. We can't test any possible combination in the universe. Why not test FileInput too etc. Again we prefer test the comunication with the Input API.
  3. It's responsibility of the maintainer future changes are properly tested.
  4. To have large test suite (even if splitted in different files) are hard to maintain and gives a false sense of tested when its not.
  5. Additional to the PHPUnit tests we rely on the coverage report. If the coverage decrease only 1 line we should investigate why.

This comment has been minimized.

Sign in to view

Copy link
@Maks3w

Maks3w Oct 8, 2015

Author Member

For reference this is similar to #40

Some feature exists sometime in the past don't means we should test all that stuf again. Its too noisy

This comment has been minimized.

Sign in to view

Copy link
@BreiteSeite

BreiteSeite Oct 8, 2015

Contributor

Thank you for taking the time to explain.

  1. BaseInputFilter its decoupled of implementations and assert resetValue is called is enough. However The behavior of resetValue its tested by the specific input test suite.
    Okay, i can see your point. However, where is the test that resetValue() is called from setData when no value is passed for an Input?
  2. Yes but that could be solved by using a mock of InputInterface instead a concrete implementation in my test.

I agree with point 3-5 but i think they have nothing to do with the discussion.

Again, thank you for your input, but i would had appreciated this feedback in my initial PR instead of silent commits that "fix" my PR after merge.

This comment has been minimized.

Sign in to view

Copy link
@Maks3w

Maks3w Oct 8, 2015

Author Member

zend-inputfilter/test/BaseInputFilterTest.php

Lines 369 to 371 in 3ffd32c

// Inputs without value must be reset for to have clean states when use different setData arguments
/** @var Input|MockObject $flatInput */
$resetInput = $this->getMockBuilder(Input::class)

{
$inputFilter = new BaseInputFilter();
$inputFilter->add(new ArrayInput(), 'foo');

$inputFilter->setData([]);
$this->assertFalse($inputFilter->get('foo')->hasValue());
}

public function addMethodArgumentsProvider()
{
$inputTypes = $this->inputProvider();
Expand Down

0 comments on commit 3ffd32c

Please sign in to comment.