FUNCTIONALITY
I have a Laravel project in which I have extended Illuminate\Database\Eloquent\Builder
as an AbstractSearch
class to introduce a params()
method that calls the abstract method applyParams()
to be defined when AbstractSearch
is extended.
Example:
$models = (new ModelSearch)
->params(['name' => 'Model', 'createdBefore' => now()])
->with(['modelCategory', 'modelType'])
->paginate();
applyParams()
is responsible for taking the array parameter from params()
and properly constructing the underlying Illuminate\Database\Query\Builder
query.
The AbstractSearch
class allows casting of these parameters via a protected $casts
attribute and a castParams(array $params = [])
method.
castParams()
loops through the protected $casts
attribute and calls $this->castParam($params, $field, $cast)
for each element.
Right now, both castParams()
and castParam()
are public methods, even though the likelihood of castParam()
ever being called that way is practically zero.
Relevent excerpt from AbstractSearch
:
public function params(array $params = [])
{
return $this->applyParams(
$this->prepareParams($params)
);
}
abstract function applyParams(array $params = []);
public function prepareParams(array $params = []) : array
{
return $this->castParams(
$this->applyDefaults($params)
);
}
public function applyDefaults(array $params = []) : array
{
foreach ($this->defaults as $field => $value) {
data_fill($params, $field, $value);
}
return $params;
}
public function getCasts()
{
return $this->casts;
}
public function castParams(array $params = []) : array
{
foreach ($this->getCasts() as $field => $cast) {
$this->castParam($params, $field, $cast);
}
return $params;
}
public function castParam(&$params, $field, $cast)
{
if (! isset($params[$field])) {
return;
}
switch ($cast) {
case 'date':
case 'datetime':
if (! $params[$field] instanceof Carbon) {
$params[$field] = Carbon::parse($params[$field]);
}
break;
default:
break;
}
}
TESTING
I have unit tests in place for both methods. The bulk of the testing is done on castParam()
to ensure that casting is done properly.
The unit test for castParams()
is done with a partial mock of the AbstractSearch
class that expects various calls to castParam()
.
QUESTIONS
I recently re-read some unit testing principles that made me question this class's setup. You can see from the excerpt above that there are a lot of public methods that should probably be protected. In fact, the only method that needs to be public is params()
.
It logically follows that params()
should be the only public method; however, params()
relies on the abstract method applyParams()
so any test of params()
would have to be done on the extending class because the results rely on what applyParams()
does.
So...
-
Have I talked myself to the right spot with the above two paragraphs? It sounds right to me, but it would involve testing that
params()
returns an instance ofIlluminate\Database\Eloquent\Builder
and that the expected changes were made to the Builder's underlyingIlluminate\Database\Query\Builder
and its query. I'm not entirely sure that's even possible. -
Going back to
castParams()
and the partial mock's expectations that it will callcastParam()
several times. If bothcastParams()
andcastParam()
remain public methods and if testing should focus on observable behavior, won't I have redundant tests if I test both? Or is it generally acceptable to mock thecastParam()
calls in mycastParams()
test sincecastParam()
will be thoroughly tested?
Aucun commentaire:
Enregistrer un commentaire