问题描述
class Meter extends Model
{
use SomeTrait;
protected self $self;
public function __construct(array $attributes = [])
{
parent::__construct($attributes);
$this->self = $this;
}
public function setSelfDI(self $self): self
{
$this->self = $self;
return $this;
}
public function calculate(): int
{
return $this->self->getDiametr() * 3.1415926;
}
public function getDiametr(): int
{
return parent::geTradius() * 2;
}
}
为了测试 calculate() 方法,我不能只调用它,因为它调用了依赖于其他特征或类的 getDiametr() 方法。 而且我认为测试模拟类是不好的做法。
所以我在我的类中注入了模拟类,并在测试过程中隔离了方法:
public function testCalculate(): void
{
/** @var Meter|MockObject $mock */
$mock = $this->createMock(Meter::class);
$mock->expects($this->once())
->method('getDiametr')
->willReturn(100);
$meter = new Meter();
$meter->setSelfDI($mock);
static::assertEquals(314.15926,$meter->calculate());
}
它非常适合我。
但最近我得到了一个代码审查,认为在函数代码之上引入合成功能 setSelfDI() 只是为了能够测试它并不是一种有效的模式。
现在我有一个我喜欢使用的普通 DI 模式,而 setSelfDI() 设置器除了用于测试之外从未使用过。
如何判断它的好坏?
解决方法
我认为测试模拟类是不好的做法。
如果您真的相信这一点,那么您已经回答了您自己的问题,因为这正是您所做的 - 您正在嘲笑类的某些方法并测试其他方法。
重要的是不要只阅读“最佳实践”的项目符号列表,而是要了解为什么提出每条规则。在这种情况下,想法是您应该有一个特定的代码“单元”来测试;应该测试由该“单元”负责的功能,并且它依赖的功能应该分离并单独测试。
这不仅仅是关于测试的建议,而是关于构建代码的建议 - 单一职责原则。
在您的示例中,更好的设计可能是以下几点之一:
- 如果半径只是数据,你不需要模拟它,创建一个已知半径的真实实例
- 如果计算半径是某个其他组件的责任,并有自己的测试,则将该对象作为依赖项传入,而不是从它继承(通常总结为“更喜欢组合而不是继承”)
- 如果计算数据不是实际代码中的输入或独立计算,请不要在测试中将其视为一项:您应该测试类的契约 - 它的输入,输出和副作用——而不是如何实现
如果 radius 是被测试类的输入,那么测试将很简单:
public function testCalculate(): void
{
$meter = new Meter(['radius' => 50);
static::assertEquals(314.15926,$meter->calculate());
}
使用组合而不是继承,您可能有这样的测试:
public function testCalculate(): void
{
// Mock the collaborating class,not the one being tested
$mock = $this->createMock(Model::class);
// Note that we're mocking getRadius,not getDiametr
// getDiametr is part of the implementation we're testing
$mock->method('getRadius')
->willReturn(50);
// Meter doesn't inherit from Model
// Instead,it requires an instance to be injected
$meter = new Meter($mock);
static::assertEquals(314.15926,$meter->calculate());
}
请注意,在这两种情况下,我们在测试中硬编码的部分都是半径 50 和结果 314.15926,而不是直径 100 - 契约测试是“给定半径 50,结果应该是 314.15926”。 calculate() 与 getDiametr() 共享代码这一事实是您履行该合同的实现。
以下实现都应该通过相同的测试。测试的重点是确保在您更改实现后,不会违反合同 - 换句话说,代码仍将执行应用程序其余部分期望它执行的操作。
// Fully shared
class Meter {
// ...
public function calculate(): int
{
return $this->getDiameter() * 3.1415926;
}
public function getDiameter(): int
{
return $this->getRadius() * 2;
}
}
// Fully copy-and-pasted
class Meter {
// ...
public function calculate(): int
{
return $this->getRadius() * 2 * 3.1415926;
}
public function getDiameter(): int
{
return $this->getRadius() * 2;
}
}
// Partially shared
class Meter {
// ...
private function getDiameterInternal(): int
{
return $this->getRadius() * 2;
}
public function calculate(): int
{
return $this->getDiameterInternal() * 3.1415926;
}
public function getDiameter(): int
{
return $this->getDiameterInternal();
}
}
// Delegated to a dependency,but still meeting the original contract
class Meter {
// ...
private $someOtherObject;
public function calculate(): int
{
return $this->someOtherObject->getDiameter() * 3.1415926;
}
public function getDiameter(): int
{
return $this->someOtherObject->getDiameter();
}
}
上述任何方法也可以在 Trait 中,它既不代表继承也不代表组合,而是“横向代码重用”,或者更简单地说“编译器辅助的复制和粘贴”。