Skip to content

Commit 3e9cd8a

Browse files
committed
(NFC) dev/core#2029 - Make assertions in PrevNextTest more skimmable
Overview -------- The test appears to have a random failure. Making it more readable may help figure out why. Before ------ `testDeleteByCacheKey()` and the related `testFillArray()` both have some assertions in these two forms: ``` // Form #1, more verbose $all = ...getSelections($cacheKey, $action); $this->assertEquals([...expected...], array_keys($all)...); // Form #2, more pithy $this->assertSelections([...expected...], $action, $cacheKey); ``` After ----- The verbose form is replaced with the pithier form. In the pithier form, some of the default inputs are made explicit. Comment ------- It is confusing that the method `getSelections()` has a parameter `$action` which can be `get` or `getall` -- and that `getall` does not (in fact) return selections. It returns all! (The fact that the contract is weird makes the unit-test helpful imho...)
1 parent eba889b commit 3e9cd8a

File tree

1 file changed

+9
-12
lines changed

1 file changed

+9
-12
lines changed

tests/phpunit/E2E/Core/PrevNextTest.php

+9-12
Original file line numberDiff line numberDiff line change
@@ -92,11 +92,10 @@ public function testFillArray() {
9292
$this->assertEquals(4, $this->prevNext->getCount($this->cacheKey));
9393
$this->assertEquals(0, $this->prevNext->getCount('not-a-key-' . $this->cacheKey));
9494

95-
$all = $this->prevNext->getSelection($this->cacheKey, 'getall')[$this->cacheKey];
96-
$this->assertEquals([100, 400, 200, 300], array_keys($all));
95+
$all = $this->assertSelections([100, 400, 200, 300], 'getall', $this->cacheKey);
9796
$this->assertEquals([1], array_unique(array_values($all)));
9897

99-
$this->assertSelections([]);
98+
$this->assertSelections([], 'get', $this->cacheKey);
10099
}
101100

102101
public function testFetch() {
@@ -250,20 +249,15 @@ public function testDeleteByCacheKey() {
250249
// Add some data that we're actually working with.
251250
$this->testFillArray();
252251

253-
$all = $this->prevNext->getSelection($this->cacheKey, 'getall')[$this->cacheKey];
254-
$this->assertEquals([100, 400, 200, 300], array_keys($all), 'selected cache not correct for ' . $this->cacheKey
255-
. ' defined keys are ' . $this->cacheKey . 'and ' . $this->cacheKeyB
256-
. ' the prevNext cache is ' . print_r($this->prevNext, TRUE)
257-
);
252+
$all = $this->assertSelections([100, 400, 200, 300], 'getall', $this->cacheKey);
258253

259254
list ($id1, $id2, $id3) = array_keys($all);
260255
$this->prevNext->markSelection($this->cacheKey, 'select', [$id1, $id3]);
261-
$this->assertSelections([$id1, $id3]);
256+
$this->assertSelections([$id1, $id3], 'get', $this->cacheKey);
262257

263258
$this->prevNext->deleteItem(NULL, $this->cacheKey);
264-
$all = $this->prevNext->getSelection($this->cacheKey, 'getall')[$this->cacheKey];
265-
$this->assertEquals([], array_keys($all));
266-
$this->assertSelections([]);
259+
$this->assertSelections([], 'getall', $this->cacheKey);
260+
$this->assertSelections([], 'get', $this->cacheKey);
267261

268262
// Ensure background data was untouched.
269263
$this->assertSelections([100], 'get', $this->cacheKeyB);
@@ -329,6 +323,8 @@ public function testDeleteAll() {
329323
* Contact IDs that should be selected.
330324
* @param string $action
331325
* @param string|NULL $cacheKey
326+
* @return array
327+
* Contact IDs that were returned by getSelection($cacheKey, $action)
332328
*/
333329
protected function assertSelections($ids, $action = 'get', $cacheKey = NULL) {
334330
if ($cacheKey === NULL) {
@@ -342,6 +338,7 @@ protected function assertSelections($ids, $action = 'get', $cacheKey = NULL) {
342338
);
343339

344340
$this->assertCount(count($ids), $selected);
341+
return $selected;
345342
}
346343

347344
}

0 commit comments

Comments
 (0)