Skip to content

Commit de1c058

Browse files
author
Ben Batschelet
committed
Remove tests for “constructor” (they were arguably bad practice, and used some API that Laravel removed)
1 parent 995df16 commit de1c058

File tree

6 files changed

+93
-148
lines changed

6 files changed

+93
-148
lines changed

Diff for: src/Relations/Indirect.php

+50-42
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ abstract class Indirect extends BelongsToMany
2727
/**
2828
* Create a new indirect relationship instance.
2929
*
30-
* @param \BeBat\PolyTree\Contracts\Node $node
31-
* @param string $foreignKey Column name that points back to this node
32-
* @param string $otherKey Column name that points to the other nodes in this relationship
30+
* @param BeBat\PolyTree\Contracts\Node $node
31+
* @param string $foreignKey Column name that points back to this node
32+
* @param string $otherKey Column name that points to the other nodes in this relationship
3333
*/
3434
public function __construct(Node $node, $foreignKey, $otherKey)
3535
{
@@ -77,9 +77,9 @@ public function isLocked()
7777
* Add a node to the indirect ancestry.
7878
*
7979
*
80-
* @param \BeBat\PolyTree\Contracts\Node $node
81-
* @param array $attributes
82-
* @param bool $touch
80+
* @param BeBat\PolyTree\Contracts\Node $node
81+
* @param array $attributes
82+
* @param bool $touch
8383
*
8484
* @throws \BeBat\PolyTree\Exceptions\LockedRelationship if this relationship has not been unlocked first
8585
*/
@@ -99,7 +99,7 @@ public function attach($node, array $attributes = [], $touch = true)
9999
* @param int|array $ids
100100
* @param bool $touch
101101
*
102-
* @throws \BeBat\PolyTree\Exceptions\LockedRelationship if this relationship has not been unlocked first
102+
* @throws BeBat\PolyTree\Exceptions\LockedRelationship if this relationship has not been unlocked first
103103
*
104104
* @return int Number of nodes detached
105105
*/
@@ -115,10 +115,10 @@ public function detach($ids = [], $touch = true)
115115
/**
116116
* Generate a query builder object that joins the ancestor ids of $parent with the descendant ids of $child.
117117
*
118-
* @param \BeBat\PolyTree\Contracts\Node $parent
119-
* @param \BeBat\PolyTree\Contracts\Node $child
118+
* @param BeBat\PolyTree\Contracts\Node $parent
119+
* @param BeBat\PolyTree\Contracts\Node $child
120120
*
121-
* @return \Illuminate\Database\Query\Builder
121+
* @return Illuminate\Database\Query\Builder
122122
*/
123123
public function getQueryForJoinedNodes(Node $parent, Node $child)
124124
{
@@ -151,10 +151,10 @@ public function getQueryForJoinedNodes(Node $parent, Node $child)
151151
/**
152152
* Generate a query that joins the id of $parent with the descendant ids of $child.
153153
*
154-
* @param \BeBat\PolyTree\Contracts\Node $parent
155-
* @param \BeBat\PolyTree\Contracts\Node $child
154+
* @param BeBat\PolyTree\Contracts\Node $parent
155+
* @param BeBat\PolyTree\Contracts\Node $child
156156
*
157-
* @return \Illuminate\Database\Query\Builder
157+
* @return Illuminate\Database\Query\Builder
158158
*/
159159
public function getQueryForParentDescendants(Node $parent, Node $child)
160160
{
@@ -175,10 +175,10 @@ public function getQueryForParentDescendants(Node $parent, Node $child)
175175
/**
176176
* Generate a query that joins the id of $child with the ancestor ids of $parent.
177177
*
178-
* @param \BeBat\PolyTree\Contracts\Node $parent
179-
* @param \BeBat\PolyTree\Contracts\Node $child
178+
* @param BeBat\PolyTree\Contracts\Node $parent
179+
* @param BeBat\PolyTree\Contracts\Node $child
180180
*
181-
* @return \Illuminate\Database\Query\Builder
181+
* @return Illuminate\Database\Query\Builder
182182
*/
183183
public function getQueryForChildAncestors(Node $parent, Node $child)
184184
{
@@ -200,10 +200,10 @@ public function getQueryForChildAncestors(Node $parent, Node $child)
200200
* Merge the ancestors of $parent and descendants of $child.
201201
*
202202
*
203-
* @param \BeBat\PolyTree\Contracts\Node $parent
204-
* @param \BeBat\PolyTree\Contracts\Node $child
203+
* @param BeBat\PolyTree\Contracts\Node $parent
204+
* @param BeBat\PolyTree\Contracts\Node $child
205205
*
206-
* @throws \BeBat\PolyTree\Exceptions\LockedRelationship if this relationship has not been unlocked first
206+
* @throws BeBat\PolyTree\Exceptions\LockedRelationship if this relationship has not been unlocked first
207207
*/
208208
public function attachAncestry(Node $parent, Node $child)
209209
{
@@ -235,12 +235,14 @@ public function attachAncestry(Node $parent, Node $child)
235235
* Appends a condition to $query for finding nodes that are either
236236
* descendants of $parent or descendants of $parent's ancestors.
237237
*
238-
* @param \Illuminate\Database\Query\Builder $query
239-
* @param \BeBat\PolyTree\Contracts\Node $parent
238+
* @param Illuminate\Database\Query\Builder $query
239+
* @param BeBat\PolyTree\Contracts\Node $parent
240+
*
241+
* @return Illuminate\Database\Query\Builder
240242
*/
241243
public function appendParentAncestorCondition(Builder $query, Node $parent)
242244
{
243-
$query->where(function ($parentQ) use ($parent) {
245+
return $query->where(function ($parentQ) use ($parent) {
244246
$parentQ->orWhere($parent->getAncestorKeyName(), $parent->getKey());
245247
$parentQ->orWhereIn($parent->getAncestorKeyName(), function ($ancestorQ) use ($parent) {
246248
$ancestorQ->select($parent->getAncestorKeyName())->from($this->getTable())
@@ -253,12 +255,14 @@ public function appendParentAncestorCondition(Builder $query, Node $parent)
253255
* Appends a condition to $query for finding nodes that are either
254256
* ancestors of $child or ancestors of $child's descendants.
255257
*
256-
* @param \Illuminate\Database\Query\Builder $query
257-
* @param \BeBat\PolyTree\Contracts\Node $child
258+
* @param Illuminate\Database\Query\Builder $query
259+
* @param BeBat\PolyTree\Contracts\Node $child
260+
*
261+
* @return Illuminate\Database\Query\Builder
258262
*/
259263
public function appendChildDescendantCondition(Builder $query, Node $child)
260264
{
261-
$query->where(function ($childQ) use ($child) {
265+
return $query->where(function ($childQ) use ($child) {
262266
$childQ->orWhere($child->getDescendantKeyName(), $child->getKey());
263267
$childQ->orWhereIn($child->getDescendantKeyName(), function ($descendantQ) use ($child) {
264268
$descendantQ->select($child->getDescendantKeyName())->from($this->getTable())
@@ -271,46 +275,50 @@ public function appendChildDescendantCondition(Builder $query, Node $child)
271275
* Appends a condition to $query for filtering out nodes that are either
272276
* $parent's direct children or descendants of those children.
273277
*
274-
* @param \Illuminate\Database\Query\Builder $query
275-
* @param \BeBat\PolyTree\Contracts\Node $parent
278+
* @param Illuminate\Database\Query\Builder $query
279+
* @param BeBat\PolyTree\Contracts\Node $parent
280+
*
281+
* @return Illuminate\Database\Query\Builder
276282
*/
277283
public function appendParentsChildrenNegation(Builder $query, Node $parent)
278284
{
279285
$parentsChildrenQ = $parent->hasChildren()->getBaseQuery()->select($parent->getKeyName());
280286

281-
$query->whereNotIn($parent->getDescendantKeyName(), $parentsChildrenQ);
282-
$query->whereNotIn($parent->getDescendantKeyName(), function ($childDescendantQ) use ($parent, $parentsChildrenQ) {
283-
$childDescendantQ->select($parent->getDescendantKeyName())->from($this->getTable())
284-
->whereIn($parent->getAncestorKeyName(), $parentsChildrenQ);
285-
});
287+
return $query->whereNotIn($parent->getDescendantKeyName(), $parentsChildrenQ)
288+
->whereNotIn($parent->getDescendantKeyName(), function ($childDescendantQ) use ($parent, $parentsChildrenQ) {
289+
$childDescendantQ->select($parent->getDescendantKeyName())->from($this->getTable())
290+
->whereIn($parent->getAncestorKeyName(), $parentsChildrenQ);
291+
});
286292
}
287293

288294
/**
289295
* Appends a condition to $query for filtering out nodes that are either
290296
* $child's direct parents or ancestors of those parents.
291297
*
292-
* @param \Illuminate\Database\Query\Builder $query
293-
* @param \BeBat\PolyTree\Contracts\Node $child
298+
* @param Illuminate\Database\Query\Builder $query
299+
* @param BeBat\PolyTree\Contracts\Node $child
300+
*
301+
* @return Illuminate\Database\Query\Builder
294302
*/
295303
public function appendChildsParentsNegation(Builder $query, Node $child)
296304
{
297305
$childsParentsQ = $child->hasParents()->getBaseQuery()->select($child->getKeyName());
298306

299-
$query->whereNotIn($child->getAncestorKeyName(), $childsParentsQ);
300-
$query->whereNotIn($child->getAncestorKeyName(), function ($parentAncestorQ) use ($child, $childsParentsQ) {
301-
$parentAncestorQ->select($child->getAncestorKeyName())->from($this->getTable())
302-
->whereIn($child->getDescendantKeyName(), $childsParentsQ);
303-
});
307+
return $query->whereNotIn($child->getAncestorKeyName(), $childsParentsQ)
308+
->whereNotIn($child->getAncestorKeyName(), function ($parentAncestorQ) use ($child, $childsParentsQ) {
309+
$parentAncestorQ->select($child->getAncestorKeyName())->from($this->getTable())
310+
->whereIn($child->getDescendantKeyName(), $childsParentsQ);
311+
});
304312
}
305313

306314
/**
307315
* Remove $parent's ancestors from $child, and $child's descendants from $parent.
308316
*
309317
*
310-
* @param \BeBat\PolyTree\Contracts\Node $parent
311-
* @param \BeBat\PolyTree\Contracts\Node $child
318+
* @param BeBat\PolyTree\Contracts\Node $parent
319+
* @param BeBat\PolyTree\Contracts\Node $child
312320
*
313-
* @throws \BeBat\PolyTree\Exceptions\LockedRelationship if this relationship has not been unlocked first
321+
* @throws BeBat\PolyTree\Exceptions\LockedRelationship if this relationship has not been unlocked first
314322
*
315323
* @return int Number of rows deleted
316324
*/

Diff for: tests/Database/IndirectAttachTest.php

+43-2
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,17 @@
22

33
namespace BeBat\PolyTree\Test\Database;
44

5+
use BeBat\PolyTree\Contracts\Node;
56
use BeBat\PolyTree\Test\TestIndirectRelation as Indirect;
67
use BeBat\PolyTree\Test\TestModel;
78
use Illuminate\Support\Collection;
89

10+
/**
11+
* Database backed tests for attaching ancestry.
12+
*
13+
* @package BeBat\PolyTree
14+
* @subpackage Test
15+
*/
916
class IndirectAttachTest extends TestCase
1017
{
1118
/**
@@ -229,6 +236,15 @@ public function testMultipleAncestorsMultipleDescendants()
229236
$this->assertAncestryMatchesFixture('attachment/expected/MultipleAncestorMultipleDescendant.yml');
230237
}
231238

239+
/**
240+
* Recursively compare values in $a & $b looking at each column in order defined by ::$orderBy
241+
*
242+
* @param array $a
243+
* @param array $b
244+
* @param int $colNum
245+
*
246+
* @return int
247+
*/
232248
protected function compareRows(array $a, array $b, $colNum = 0)
233249
{
234250
if (!isset($this->orderBy[$colNum])) {
@@ -246,13 +262,23 @@ protected function compareRows(array $a, array $b, $colNum = 0)
246262
return $this->compareRows($a, $b, $colNum + 1);
247263
}
248264

265+
/**
266+
* Add the $rows as an array dataset to the ancestry table.
267+
*
268+
* @param array $rows
269+
*/
249270
protected function addRowsToAncestry(array $rows)
250271
{
251272
$existingAncestry = $this->createSimpleArrayDataSet('node_ancestry', $rows);
252273

253274
$this->appendDataSet($existingAncestry);
254275
}
255276

277+
/**
278+
* Assert that the data in ancestry table matches $rows
279+
*
280+
* @param array $rows
281+
*/
256282
protected function assertAncestryMatchesArray(array $rows)
257283
{
258284
usort($rows, [$this, 'compareRows']);
@@ -264,6 +290,11 @@ protected function assertAncestryMatchesArray(array $rows)
264290
$this->assertTablesEqual($expected, $actual);
265291
}
266292

293+
/**
294+
* Assert that the data in ancestry table matches a given YAML fixture.
295+
*
296+
* @param string $path
297+
*/
267298
protected function assertAncestryMatchesFixture($path)
268299
{
269300
$expected = $this->createYamlDataSet($path)->getTable('node_ancestry');
@@ -273,9 +304,19 @@ protected function assertAncestryMatchesFixture($path)
273304
$this->assertTablesEqual($expected, $actual);
274305
}
275306

307+
/**
308+
* Check various query building functions return the expected results for $parent and $child.
309+
*
310+
* @param BeBat\PolyTree\Contracts\Node $parent
311+
* @param BeBat\PolyTree\Contracts\Node $child
312+
* @param array $expJoined Expected IDs cross joined between
313+
* $parent's ancestors and $child's descendants
314+
* @param array $expChildAncestors Expected IDs with $parent's ancestors and $child
315+
* @param array $expParentDescendants Expected IDs with $parent and $child's descendants
316+
*/
276317
protected function doAttachmentAssertions(
277-
TestModel $parent,
278-
TestModel $child,
318+
Node $parent,
319+
Node $child,
279320
array $expJoined = [],
280321
array $expChildAncestors = [],
281322
array $expParentDescendants = []

Diff for: tests/Relations/HasAncestorsTest.php

-10
Original file line numberDiff line numberDiff line change
@@ -98,16 +98,6 @@ public function tearDown()
9898
Mockery::close();
9999
}
100100

101-
/**
102-
* Test that parameters are forwarded to the parent constructor correctly.
103-
*/
104-
public function testConstructor()
105-
{
106-
// Format is [table].[key_name] and we only care about the key name
107-
verify('foreign key is descendant', $this->relation->getForeignKey())->endsWith('.descendant_key_name');
108-
verify('other key is ancestor', $this->relation->getOtherKey())->endsWith('.ancestor_key_name');
109-
}
110-
111101
/**
112102
* Test that attach() throws an exception if a cycle would be created.
113103
*/

Diff for: tests/Relations/HasChildrenTest.php

-42
This file was deleted.

Diff for: tests/Relations/HasDescendantsTest.php

-10
Original file line numberDiff line numberDiff line change
@@ -97,16 +97,6 @@ public function tearDown()
9797
Mockery::close();
9898
}
9999

100-
/**
101-
* Test that parameters are forwarded to the parent constructor correctly.
102-
*/
103-
public function testConstructor()
104-
{
105-
// Format is [table].[key_name] and we only care about the key name
106-
verify('foreign key is ancestor', $this->relation->getForeignKey())->endsWith('.ancestor_key_name');
107-
verify('other key is descendant', $this->relation->getOtherKey())->endsWith('.descendant_key_name');
108-
}
109-
110100
/**
111101
* Test that attach() throws an exception if a cycle would be created.
112102
*/

0 commit comments

Comments
 (0)