-
Notifications
You must be signed in to change notification settings - Fork 69
test enhancement #62
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
base: master
Are you sure you want to change the base?
test enhancement #62
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,11 @@ | ||
| dist: trusty | ||
| language: php | ||
|
|
||
| php: | ||
| - 7.1 | ||
| - 7.2 | ||
|
|
||
| install: | ||
| - composer install --prefer-dist | ||
| - composer install | ||
|
|
||
| script: | ||
| - vendor/bin/phpunit | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,4 +2,6 @@ | |
|
|
||
| require __DIR__ . '/iter.fn.php'; | ||
| require __DIR__ . '/iter.php'; | ||
| require __DIR__ . '/iter.rewindable.php'; | ||
| require __DIR__ . '/iter.rewindable.php'; | ||
| require __DIR__ . '/../test/MyIterator.php'; | ||
| require __DIR__ . '/../test/MyIteratorAgg.php'; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These files shouldn't be included here, as they are only needed for tests. Either we should have a separate bootstrap file in the test directory, or we can explicitly include the files in the test, or we can place the class definitions in the test file (like
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All right, I will move the related
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I use the |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| <?php | ||
|
|
||
| namespace iter; | ||
|
|
||
| class MyIterator implements \Iterator { | ||
| private $position = 0; | ||
| private $array = [ | ||
| "firstelement", | ||
| "secondelement", | ||
| "lastelement", | ||
| ]; | ||
|
|
||
| public function __construct() { | ||
| $this->position = 0; | ||
| } | ||
|
|
||
| public function rewind() { | ||
| $this->position = 0; | ||
| } | ||
|
|
||
| public function current() { | ||
| return $this->array[$this->position]; | ||
| } | ||
|
|
||
| public function key() { | ||
| return $this->position; | ||
| } | ||
|
|
||
| public function next() { | ||
| ++$this->position; | ||
| } | ||
|
|
||
| public function valid() { | ||
| return isset($this->array[$this->position]); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| <?php | ||
|
|
||
| namespace iter; | ||
|
|
||
| class MyIteratorAgg implements \IteratorAggregate { | ||
| public $property1 = "Public property one"; | ||
| public $property2 = "Public property two"; | ||
| public $property3 = "Public property three"; | ||
|
|
||
| public function __construct() { | ||
| $this->property4 = "last property"; | ||
| } | ||
|
|
||
| public function getIterator() { | ||
| return new \ArrayIterator($this); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -359,6 +359,10 @@ public function testCount() { | |
| $this->assertSame(42, count(new _CountableTestDummy)); | ||
| } | ||
|
|
||
| public function testCountWithStringCount() { | ||
| $this->assertSame(3, count(new MyIterator())); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really understand the purpose of this test. This is testing counting of an iterator, which is also tested by the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's my fault to create the duplicated tests. |
||
| } | ||
|
|
||
| public function testIsEmpty() { | ||
| $this->assertTrue(isEmpty([])); | ||
| $this->assertFalse(isEmpty([null])); | ||
|
|
@@ -368,6 +372,10 @@ public function testIsEmpty() { | |
| $this->assertFalse(isEmpty(repeat(42))); | ||
| } | ||
|
|
||
| public function testIsEmptyWithValidIteratorAgg() { | ||
| $this->assertFalse(isEmpty(new MyIteratorAgg())); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess we should test |
||
| } | ||
|
|
||
| public function testToArray() { | ||
| $this->assertSame([1, 2, 3], toArray(['a' => 1, 'b' => 2, 'c' => 3])); | ||
| $this->assertSame( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about moving this to
composer.jsonvia?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@localheinz, thank you for your reply.
It looks like adding this
--prefer-distconfig incomposer.jsoninstead of letting this be the option incomposer installcommand.I think revert this option in
composer installcommand is the proper way if this option is required.What do you think? Thanks.