Skip to content
Snippets Groups Projects
Commit 3e9cd8a8 authored by totten's avatar totten
Browse files

(NFC) #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...)
parent eba889b0
Branches
Tags
No related merge requests found
......@@ -92,11 +92,10 @@ class PrevNextTest extends \CiviEndToEndTestCase {
$this->assertEquals(4, $this->prevNext->getCount($this->cacheKey));
$this->assertEquals(0, $this->prevNext->getCount('not-a-key-' . $this->cacheKey));
$all = $this->prevNext->getSelection($this->cacheKey, 'getall')[$this->cacheKey];
$this->assertEquals([100, 400, 200, 300], array_keys($all));
$all = $this->assertSelections([100, 400, 200, 300], 'getall', $this->cacheKey);
$this->assertEquals([1], array_unique(array_values($all)));
$this->assertSelections([]);
$this->assertSelections([], 'get', $this->cacheKey);
}
public function testFetch() {
......@@ -250,20 +249,15 @@ class PrevNextTest extends \CiviEndToEndTestCase {
// Add some data that we're actually working with.
$this->testFillArray();
$all = $this->prevNext->getSelection($this->cacheKey, 'getall')[$this->cacheKey];
$this->assertEquals([100, 400, 200, 300], array_keys($all), 'selected cache not correct for ' . $this->cacheKey
. ' defined keys are ' . $this->cacheKey . 'and ' . $this->cacheKeyB
. ' the prevNext cache is ' . print_r($this->prevNext, TRUE)
);
$all = $this->assertSelections([100, 400, 200, 300], 'getall', $this->cacheKey);
list ($id1, $id2, $id3) = array_keys($all);
$this->prevNext->markSelection($this->cacheKey, 'select', [$id1, $id3]);
$this->assertSelections([$id1, $id3]);
$this->assertSelections([$id1, $id3], 'get', $this->cacheKey);
$this->prevNext->deleteItem(NULL, $this->cacheKey);
$all = $this->prevNext->getSelection($this->cacheKey, 'getall')[$this->cacheKey];
$this->assertEquals([], array_keys($all));
$this->assertSelections([]);
$this->assertSelections([], 'getall', $this->cacheKey);
$this->assertSelections([], 'get', $this->cacheKey);
// Ensure background data was untouched.
$this->assertSelections([100], 'get', $this->cacheKeyB);
......@@ -329,6 +323,8 @@ class PrevNextTest extends \CiviEndToEndTestCase {
* Contact IDs that should be selected.
* @param string $action
* @param string|NULL $cacheKey
* @return array
* Contact IDs that were returned by getSelection($cacheKey, $action)
*/
protected function assertSelections($ids, $action = 'get', $cacheKey = NULL) {
if ($cacheKey === NULL) {
......@@ -342,6 +338,7 @@ class PrevNextTest extends \CiviEndToEndTestCase {
);
$this->assertCount(count($ids), $selected);
return $selected;
}
}
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment