Code that is 'unit testable' means that it has been written with testing its features in mind. Code that is not 'unit testable' is often called 'happy path' code which makes an assumption that all values and objects passed to it are valid and not null.
In school we all heard the old adage:
Q: "what's the first thing you do when writing a software program that peels apples?"
A: "check to make sure you're holding an apple"
That means checking:
Is the object null?
Does it implement the interface you require?
Is it an instance of the object you are expecting?
Taking it further, we need to utilize Laravel's 'implicit loading' feature as often as possible.
What Is Implicit Loading?
Implicit Loading is automatically loading an object based on the ID passed in the URI. eg:
/lotteries/{lottery_id}/stages/{stage_id} //this will not fire implicit loading
This will dictate that the controller's method looks like:
public function doSomething(int $lottery_id, int $stage_id) {
//code here
}
The problem with this is we still need to verify that the lottery exists for the lottery_id passed in. We also need to verify that the stage exists for the stage_id passed in. that means we need to add this inside our controller:
public function doSomething(int $lottery_id, int $stage_id) {
$lottery = Lottery::where('lottery_id', $lottery_id)->first();
if(is_null($lottery)) {
throw new ObjectNotFoundException('No lottery exists for id ' . $lottery_id);
}
$stage = Stage::where('stage_id', $stage_id)->first();
if(is_null($stage)) {
throw new ObjectNotFoundException('No stage exists for id ' . $stage_id);
}
}
Implicit loading will take care of the loading of objects BEFORE the execution of code reaches the controller and throw an exception if they are not found.
What developers often do is:
public function saveStage(SaveStageRequest $request, int $lottery_id, int $stage_id) {
Stage::updateOrCreate($request->validated());
// return ResponseObject...
}
This is the happy path - assuming that everything is ready to save, not considering an exception that could occur saving to the database, and sending a success only response. This is not code that we can write unit tests for, apart from a basic unit test of: public function testSave_success()
We need to write software that puts the onus on the requester to prove to us that all conditions are pristine before we will consider the request. Following the example of 'save a stage to a lottery':
/lotteries/{lottery}/stages/{stage} //this WILL fire implicit loading
This will dictate that the controller's method looks like:
public function doSomething(Lottery $lottery, Stage $stage) {
//code here
}
This has already proven to the code that the values passed back in the URI are valid rows in the database to work with. This is only the start though.
Other aspects to consider:
Does that stage map to the lottery or does it map to a different lottery?
We've only validated their existence in the database.
EVEN IF YOU AREN'T USING THE OBJECT IN THE CODE YOU SHOULD RELY ON IMPLICIT LOADING FOR YOUR VALIDATION CHECKS
We aren't needing the Lottery object simply to update the values of a stage. But we do need to ensure they map to each other:
if($lottery->lottery_id != $stage->lottery_id) {
throw new ObjectMappingException('Stage does not map to posted lottery ID ' . $lottery->lottery_id);
}
Stage also maps to:
Jobs (submissions)
Status
These need to be verified before we attempt to insert into the database. The limited validation being utilized by Laravel only verifies that they are in fact valid integers and whether or not they are required.
That means our pseduocode might look like this:
const INVALID_LOTTERY_STAGE_MAPPING_EXCEPTION_MESSAGE = 'Invalid lottery/stage mapping';
const INVALID_SUBMISSION_MAPPING_EXCEPTION_MESSAGE = 'Invalid submission mapping';
const INVALID_STATUS_MAPPING_EXCEPTION_MESSAGE = 'Invalid status mapping';
public function doSomething(Lottery $lottery, Stage $stage) {
try{
if($lottery->lottery_id != $stage->lottery_id) {
throw new ObjectMappingException(
INVALID_LOTTERY_STAGE_MAPPING_EXCEPTION_MESSAGE
);
}
$submission = Submission::where('submission_id', $stage->submission_id)->first();
if(is_null($submission)) {
throw new ObjectMappingException(
INVALID_SUBMISSION_MAPPING_EXCEPTION_MESSAGE
);
}
//status is a string - we need to check to ensure it's an expected string
if(!in_array($stage->status,
[
Stage::STATUS_OPTIONS_REQUIRED,
Stage::STATUS_READY,
Stage::STATUS_OPEN,
Stage::STATUS_CLOSED,
Stage::STATUS_MERGED,
Stage::STATUS_DRAFT_IN_PROGRESS,
Stage::STATUS_DRAFT_FAILED,
Stage::STATUS_DRAFT_SUCCEEDED
]
) {
throw new ObjectMappingException(
INVALID_STATUS_MAPPING_EXCEPTION_MESSAGE
);
}
//now we can execute the request
//code to execute goes here...
}catch(\Exception $exception) {
return (new EnvelopedResponse)
->setStatus(ResponseStatus::ERROR)
->setData($exception->getMessage());
}
}
Often there is more code trying to prove that a request cannot be honored that the code required to honor the request. This is good - this is to prove that the conditions are pristine to ensure that an exception does not occur during execution of the request, where values may be generated or registered in the processing of a request and then an exception occurs mid way through the request - leaving orphaned values in data stores unless proper transactions and rollbacks are implemented - another aspect of the software that is all too often not taken into consideration.
By checking for pristine conditions before the execution of code we mitigate the need for rollbacks (not all rollbacks can be done within a basic DB transaction).
Writing a Unit Test for Testable Code
Now that we've implemented as many possible checks for pristine conditions we need to test EACH of the IF statements (not just check the exceptions thrown).
public function testBasicSave_shouldReturnSuccess() {
$lottery = LotteriesSeedData::getBaseLottery(); //previously mapped object in seed data
$stage = StagesSeedData::getBaseStage(); //previously mapped object in seed data
$response = $stagesController->doSomething($lottery, $stage);
//check the response status, not the string values that can change dynamically
$this-assertEquals($response->getStatusCode() == ResponseStatus::SUCCESS);
}
public function testInvalidLotteryStageMapping_shouldReturnException() {
$lottery = LotteriesSeedData::getInvalidLottery(); //previously mapped object in seed data
$stage = StagesSeedData::getBaseStage(); //previously mapped object in seed data
$response = $stagesController->doSomething($lottery, $stage);
//check we have an exception
$this-assertEquals($response->getStatusCode() == ResponseStatus::ERROR);
//check to make sure it's the correct exception
$this->assertEquals($response->getData() == LotteriesController::INVALID_LOTTERY_STAGE_MAPPING_EXCEPTION_MESSAGE);
}
public function testInvalidSubmission_shouldReturnException() {
$lottery = LotteriesSeedData::getBaseLottery(); //previously mapped object in seed data
$stage = StagesSeedData::getBaseStage(); //previously mapped object in seed data
$stage->submission_id = StagesSeedData::getInvalidSubmmitionId();
$response = $stagesController->doSomething($lottery, $stage);
//check we have an exception
$this-assertEquals($response->getStatusCode() == ResponseStatus::ERROR);
//check to make sure it's the correct exception
$this->assertEquals($response->getData() == LotteriesController::INVALID_SUBMISSION_MAPPING_EXCEPTION_MESSAGE);
}
public function testInvalidStatus_shouldReturnException() {
$lottery = LotteriesSeedData::getBaseLottery(); //previously mapped object in seed data
$stage = StagesSeedData::getBaseStage(); //previously mapped object in seed data
$stage->phase = 'some random junk';
$response = $stagesController->doSomething($lottery, $stage);
//check we have an exception
$this-assertEquals($response->getStatusCode() == ResponseStatus::ERROR);
//check to make sure it's the correct exception
$this->assertEquals($response->getData() == LotteriesController::INVALID_STATUS_MAPPING_EXCEPTION_MESSAGE);
}
Checking All If Statements
With unit testing we have to check all paths within our methods we test.
if($value == self::SOME_INITIAL_VALUE) {
//do something here
} else if($value == self::SOME_OTHER_VALUE) {
//do something here
} else if($value == self::YET_SOME_OTHER_VALUE) {
//do something here
}
There should be a provable test for each of these conditions - that means 3 separate tests. Every line of code should have test coverage which means:
every line of comparison (if/else)
every exception
every method call
every controller endpoint
every service method ~ public, protected, and private
Writing Code That Is Testable
Separation of Concerns Avoid writing an endpoint that puts all the code into 1 method inside a controller. The controller is simply meant to determine what needs to be done, and should not do the work itself.
Pass the work off to one or more associated services to do the work. Even refactoring an endpoint to utilize services takes an average of 15 minutes to refactor, including writing the service, the interface, registering both of them, and then refactoring the code. It is not a very challenging task.
Separating each portion of a request into a service makes it easily unit testable. If a method is doing more than 1 task it should be refactored into a method for each task.
Sample of proving all conditions are pristine before handling request:
const INVALID_LOTTERY_STATE_EXCEPTION_MESSAGE = 'Invalid lottery state returned for {lotteryId}';
const INVALID_DRAFT_STATE_EXCEPTION_MESSAGE = 'Invalid Draft state returned for {scheduleDraftId}';
const NO_STAGES_FOUND_EXCEPTION_MESSAGE = 'No stages found for lottery';
const INVALID_STAGE_EXCEPTION_MESSAGE = 'Invalid Stage state "{status}' returned for {stageId}";
/**
* @param PublishLotteryRequest $request
* @param Lottery $lottery
* @return EnvelopedResponse
* @throws InvalidStatusException
* @throws StageException
* @throws \Illuminate\Auth\Access\AuthorizationException
*/
public function publish(PublishLotteryRequest $request, Lottery $lottery) {
$this->authorize('create', new Lottery());
try {
//check the status of the lottery to avoid generating a duplicate schedule
if ($lottery->status != Lottery::STATUS_DRAFT) {
throw new InvalidStatusException(sprintf(NO_STAGES_FOUND_EXCEPTION_MESSAGE,$lottery->lottery_id));
}
//check the status of the draft
$draft = $lottery->draft()->first();
if (is_null($draft) || $draft->status != ScheduleDraft::STATUS_DRAFT) {
throw new InvalidStatusException(sprintf(INVALID_DRAFT_STATE_EXCEPTION_MESSAGE, $lottery->schedule_draft_id));
}
//check to ensure each stage is complete
$stages = $lottery->stages()->get();
if (count($stages) == 0) {
throw new StageException('No stages found for lottery');
}
foreach ($stages as $stage) {
if ($stage->status != Stage::STATUS_CLOSED) {
throw new InvalidStatusException(sprintf(INVALID_STAGE_EXCEPTION_MESSAGE, $stage->status, $stage->stage_id));
}
}
//still here? publish the lottery
//THIS IS THE ONE LINE OF CODE THAT ACTUALLY HANDLES THE PUBLISH REQUEST
$this->service->publish($lottery);
return (new EnvelopedResponse)
->setStatus(ResponseStatus::SUCCESS)
->setData(PublishLotteryResource::make(
$lottery
));
}catch (\Exception $exception) {
return (new EnvelopedResponse)
->setStatus(ResponseStatus::ERROR)
->setData($exception->getMessage());
}
}
Matching Integration unit tests would be:
private $lotteriesController;
public function setUp(): void
{
parent::setUp();
$this->lotteriesController = $this->app->make('Entrada\Modules\Lotteries\Controllers\LotteryController');
}
/**
* @test
* @return void
*/
public function testBasicPublish_shouldReturn200() {
$lottery = LotteriesSeedData::getBaseLottery();
$result = $this->lotteriesController->publish($request, $lottery);
$this->assertEquals($result->getResponseStatus(), ResponseStatus::SUCCESS);
}
/**
* @test
* @return void
*/
public function testPublishInvalidLotteryStatus_shouldReturnException() {
$lottery = LotteriesSeedData::getBasePublishedLottery(); //requires a 2nd lottery with status != draft
$result = $this->lotteriesController->publish($request, $lottery);
$this->assertEquals($result->getResponseStatus(), ResponseStatus::ERROR);
$this->assertEquals($result->getData(), sprintf(LotteryController::INVALID_LOTTERY_STATE_EXCEPTION_MESSAGE, $lottery->lottery_id));
}
/**
* @test
* @return void
*/
public function testPublishInvalidDraftState_shouldReturnException() {
$lottery = LotteriesSeedData::getBaseLotteryDraftStatusPublished(); //requires a 3rd lottery with draft status != draft
$result = $this->lotteriesController->publish($request, $lottery);
$this->assertEquals($result->getResponseStatus(), ResponseStatus::ERROR);
$this->assertEquals($result->getData(), sprintf(LotteryController::INVALID_DRAFT_STATE_EXCEPTION_MESSAGE, $lottery->schedule_draft_id));
}
/**
* @test
* @return void
*/
public function testPublishStageNotFound_shouldReturnException() {
$lottery = LotteriesSeedData::getBaseLotteryNoStages(); //requires a 4th lottery with no stages set in seed data
$result = $this->lotteriesController->publish($request, $lottery);
$this->assertEquals($result->getResponseStatus(), ResponseStatus::ERROR);
$this->assertEquals($result->getData(), LotteryController::NO_STAGES_FOUND_EXCEPTION_MESSAGE);
}
/**
* @test
* @return void
*/
public function testPublishInvalidStageState_shouldReturnException() {
$lottery = LotteriesSeedData::getBaseLotteryWithStageClosed(); //requires a 4th lottery with no stages set in seed data
$invalidStage = StagesSeedData::getInvalidStatusStage();
$result = $this->lotteriesController->publish($request, $lottery);
$this->assertEquals($result->getResponseStatus(), ResponseStatus::ERROR);
$this->assertEquals($result->getData(), sprintf(LotteryController::INVALID_STAGE_EXCEPTION_MESSAGE, $invalidStage->status, $invalidStage->stage_id));
}
Even inside the injected service, further checks are performed:
const NO_SCHEDULE_ASSIGNMENTS_EXCEPTION_MESSAGE = 'No schedule assignments found for lottery {lotteryId}';
/**
* publishes a lottery
*
* @param Lottery $lottery
* @throws LotteryException
*/
public function publish(Lottery $lottery) {
$scheduleAssignments = ScheduleAssignment::whereIn('stage_id', $lottery->stages()->get())->get();
if($scheduleAssignments->count() == 0) {
throw new LotteryException('No schedule assignments found for lottery ' . $lottery->lottery_id);
}
foreach($scheduleAssignments as $scheduleAssignment ) {
ScheduleAudience::create(
[
'schedule_id' => $scheduleAssignment->block_id,
'schedule_slot_id' => $scheduleAssignment->slot_id,
'audience_type' => ScheduleAudience::AUDIENCE_TYPE_USER,
'audience_value' => $scheduleAssignment->proxy_id
]
);
}
//set the lottery to published so it cannot be posted to again
$lottery->status = Lottery::STATUS_PUBLISHED;
$lottery->update();
}
Matching Integration unit tests would be:
private $lotteriesService;
public function setUp(): void
{
parent::setUp();
$this->locationsService = $this->app->make('Entrada\Modules\Lotteries\Services\LotteriesService');
}
/**
* @test
* @return void
*/
public function testBasicPublish_shouldReturnVoid() {
$lottery = LotteriesSeedData::getBaseLottery();
$result = $this->lotteriesService->publish($lottery);
$this->assertEquals($result->getStatusCode(), ResponseCode::SUCCESS);
}
/**
* @test
* @return void
*/
public function testNoScheduleAssignments_shouldReturnException() {
$lottery = LotteriesSeedData::getBaseLotteryNoScheduleAssignments();
$result = $this->lotteriesService->publish($lottery);
$this->assertEquals($result->getStatusCode(), ResponseCode::FAIL);
$this->assertEquals($result->getData(), sprintf(LotteryService::NO_SCHEDULE_ASSIGNMENTS_EXCEPTION_MESSAGE, $lottery->lottery_id));
}
