Categorias
Solving Problems

Solving problems week 2: Cypress test automation, E2E, DevExp, code standards, rector

In this week's Solving Problems text, I have two topics: code quality and service reliability. I will share some ideas that can improve your set of tools that check if your codebase is healthy.

The problem with the Code Quality topic is how to keep your standards and some trivial issues out of the code without sacrificing your reviewer's time and patience.

And the problem with Service Reliability is how to be active in monitoring the most essential parts of your service without relying on human tests.

Automated tests

There are some red lights that you might be missing an important part of your Software Reliability: the QA team being a bottleneck due to human-resource timing constraints, too many PR reverts before deployment and lack of unit tests on each codebase. That usually comes with a very concerning outcome: customer support tickets. That is very bad for the product's reputation and a constant source of stress and bug lists to grow.

There are lots of quality checks that we could talk about, like enforcing unit testing coverage in the pipeline, applying feature flagging, improving the regression QA processing steps, etc. They are all pivotal and needed, but where to start? There is a first step, on similar scenarios, that I recommend prioritised, keeping the other processes running in parallel: automated end-to-end tests.

Testing plays a key role in development. By continuously monitoring application workflows and features, your tests can surface broken functionality before your customers do.

-- Best practices for creating end-to-end tests, DataDog

An Automated end-to-end test (we can also call it Smoke tests) must cover your most important scenarios and test them continuously and after any deployment. After you check those most four/five very important scenarios, you can keep improving the smoke tests based on the most used use cases. We have currently powerful tools to write them. Let's say cypress

describe('Verify dashboard', () => {
    const baseUrl = ${Cypress.env('baseUrl')};
    const env = Cypress.env('env');
    const dataFile: any = credentials_${env}.json;

    it('Verify raw Admin User profile', () => {
        cy.loginApplication();
        cy.fixture(dataFile).then(testData => {
            const profileUrl = testData['adminUser'].profileUrl;
            cy.visit(baseUrl + profileUrl);
        });
        cy.contains('Profile');
        cy.visit(${baseUrl}/logout);
    });
});

Developer experience

A quality code check that a mature codebase has is to lint and check code standards. As authors, we are used to waiting for CI to perform steps and be sure that the same successful result status we see when running the steps locally is also successful in CI. As reviewers, we are used to comments asking to check CI or asking to use the agreed pattern when the codebase doesn't have a good quality check step in place.

Both are part of the passive code review that steals our time from the essential problems we need to review in the code: architectural or business logic problems that are often missed by tired eyes. We can do better and use CI and the step tool in our favour.

This can be reproduced in any language, but focusing on PHP, we can use tools like Rector to check and fix those easy-to-spot problems. You can just set a step in our pipeline that will fix the errors and commit it again.

Some can say that it could be a pre-hook step, but this is usually skipped when takes more than 2s. I agree that this is easy to just run the fixes on the diff in our local, but we just have to do better if we automate the changes in case some developers just do not run it before pushing the commits or whatever reason.

This would be a very useful automation, that will run for every open Pull Request, committing code standards or lint issues. The pipeline will contribute a lot to your codebase with very little maintenance. Mind that this will execute Rector (therefore moving the code to the state your team agreed and not just code style) and Easy Code Standards (combine power of PHP_CodeSniffer and PHP CS Fixer in 3 lines)

name: Rector CI

on:
  pull_request: null

jobs:
  rector-ci:
    runs-on: ubuntu-latest
    # run only on commits on main repository, not on forks
    if: github.event.pull_request.head.repo.full_name == github.repository
    env:
      COMPOSER_AUTH: ${{ secrets.COMPOSER_AUTH }}
    steps:
      - uses: actions/checkout@v4
        with:
          # Solves the not "You are not currently on a branch" problem, see https://github.com/actions/checkout/issues/124#issuecomment-586664611
          ref: ${{ github.event.pull_request.head.ref }}
          # Must be used to trigger workflow after push
          token: ${{ secrets.GH_PAT_TOKEN }}

      - uses: shivammathur/setup-php@v2
        with:
          php-version: 8.1
          coverage: none
          extensions: <your-extensions-here>

      -   run: composer install --no-progress --ansi

      ## First run Rector without --dry-run, it would stop the process with exit 1 here
      -   run: vendor/bin/rector process --ansi

      - name: Check for Rector modified files
        id: rector-git-check
        run: |
          export CHANGES=$(if git diff --exit-code --no-patch; then echo "false"; else echo "true"; fi)
          echo "modified=$CHANGES" >> "$GITHUB_OUTPUT"

      - name: Git config
        if: steps.rector-git-check.outputs.modified == 'true'
        run: |
          git config --global user.name 'rector-bot'
          git config --global user.email '[email protected]'
          export LOG=$(git log -1 --pretty=format:"%s")
          echo "COMMIT_MESSAGE=${LOG}" >> "$GITHUB_ENV"

      - name: Commit Rector changes
        if: steps.rector-git-check.outputs.modified == 'true'
        run: git commit -am "[rector] ${COMMIT_MESSAGE}"

      ## Now, there might be coding standard issues after running Rector
      - run: composer run ecs:fix

      - name: Check for CS modified files
        id: cs-git-check
        run: |
          export CHANGES=$(if git diff --exit-code --no-patch; then echo "false"; else echo "true"; fi)
          echo "modified=$CHANGES" >> "$GITHUB_OUTPUT"

      - name: Git config
        if: steps.cs-git-check.outputs.modified == 'true'
        run: |
          git config --global user.name 'rector-bot'
          git config --global user.email '[email protected]'
          export LOG=$(git log -1 --pretty=format:"%s")
          echo "COMMIT_MESSAGE=${LOG}" >> "$GITHUB_ENV"

      - name: Commit CS changes
        if: steps.cs-git-check.outputs.modified == 'true'
        run: git commit -am "[cs] ${COMMIT_MESSAGE}"

      - name: Push changes
        if: steps.cs-git-check.outputs.modified == 'true'
        run: git push

Links:

Categorias
PHP

PHP 8.1: more on new in initializers

I could not agree more with Brent when he says concerning the "new in initializers"[1] feature:

PHP 8.1 adds a feature that might seem like a small detail, but one that I think will have a significant day-by-day impact on many people.

When I see this new feature, lots of places that use Dependency Injection[3] come to my focus as candidates to be impacted, such as application or infrastructure service classes. As a result, we will write a much cleaner and leaner code without giving up on good practices to write modular, maintainable and testable software.

The Dependency Inversion Principle[4] gives us decoupling powers. But we know that many classes will receive the same concrete implementation most of (if not all) the time.

So this is very common to see some variation of this code:

$someDependencyToBeInjected = FactoryClass::create();
$someService = new SomeServiceClass($someDependencyToBeInjected);

Important note: I will ignore for now Service Containers and frameworks features that deal with service instantiation, auto wiring, etc.

Think of a database query service class: you depend on a connection object. Every time you need to instantiate your database service class, you need to prepare the connection dependency and inject it at the service class. Database connections are a great example when you use the same concrete implementation more than 90% of the time.

The same applies to a Service class that you use to handle business logic and depends on QueryService and CommandHandler interfaces to do its job.

Before PHP 8.1 we have code like this:

// service class to apply business logic
// most standard
class DefaultLeadRecordService implements LeadRecordService
{
    public function __construct(
        private LeadQueryService $queryService,
        private LeadCommandHandler $commandHandler
    ) {
    }
}

// infrastructure class to match the interface -- a DBAL concrete class
// sneakily allowing a "default" value, but also open to Dependency Injection
// but not that great
class DbalLeadQueryService implements LeadQueryService
{
    public function __construct(private ?Connection $connection = null)
    {
        if (!$this->connection) {
            $this->connection = Core::getConnection();
        }
    }
}

// instantiation would be something like -- given you have $connection already instantiated
$connection =  \Doctrine\DBAL\DriverManager::getConnection($connectionParams, $config);

$service = new \Blog\Application\DefaultLeadRecordService(
    new \Blog\Infrastructure\DbalLeadQueryService($connection),
    new \Blog\Infrastructure\DbalLeadCommandHandler($connection),
);

// if we allow construct get default value
$service = new \Blog\Application\DefaultLeadRecordService(
    new \Blog\Infrastructure\DbalLeadQueryService(),
    new \Blog\Infrastructure\DbalLeadCommandHandler(),
);

While on PHP 8.1, you will be able to write it like so:

class DefaultLeadRecordService implements LeadRecordService
{
    public function __construct(
        private LeadQueryService $queryService = new DbalLeadQueryService(),
        private LeadCommandHandler $commandHandler = new DbalLeadCommandHandler()
    ) {
    }
}

// we see there is still room for new features here
// still not that great
class DbalLeadQueryService implements LeadQueryService
{
    public function __construct(private ?Connection $connection = null)
    {
        // waiting when new initializers feature allows static function as default parameters
        if (!$this->connection) {
            $this->connection = Core::getConnection();
        }
    }
}

$service = new \Blog\Application\DefaultLeadRecordService();

One liner! That saves a lot of typing, and the code remains very well structured. This is the type of significant impact we will have on our day-to-day work. We will write a more straightforward, robust and meaningful code, and we will ship features faster with high-quality code.

If you want to see a full implementation of this, check the code at https://github.com/rafaelbernard/blog-php-81-new-initializers

Test, our faithful friend

Writing tests is a must-have for any repository where quality is a requirement. However, the "New in initializers" feature does not force us to give up on a complete suite of tests. We still have all powers of unit or integration tests.

For application code, we would write unit tests and all the expectations for concrete dependencies:

<?php

namespace Test\Unit\Blog\Application;

use Blog\Application\DefaultLeadRecordService;
use Blog\Domain\LeadCommandHandler;
use Blog\Domain\LeadQueryService;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;

class DefaultLeadRecordServiceTest extends TestCase
{
    private const EMAIL = '[email protected]';

    private LeadQueryService|MockObject $leadQueryServiceMock;
    private LeadCommandHandler|MockObject $leadCommandHandlerMock;

    private DefaultLeadRecordService $service;

    protected function setUp(): void
    {
        parent::setUp();

        $this->leadQueryServiceMock = $this->getMockBuilder(LeadQueryService::class)->getMock();
        $this->leadCommandHandlerMock = $this->getMockBuilder(LeadCommandHandler::class)->getMock();

        $this->service = new DefaultLeadRecordService($this->leadQueryServiceMock, $this->leadCommandHandlerMock);
    }

    public function testCanAdd()
    {
        $this->leadQueryServiceMock
            ->expects(self::once())
            ->method('getByEmail')
            ->with(self::EMAIL)
            ->willReturn(false);

        $this->leadCommandHandlerMock
            ->expects(self::once())
            ->method('add')
            ->with(self::EMAIL)
            ->willReturn(1);

        $result = $this->service->add(self::EMAIL);

        self::assertEquals(1, $result);
    }

    public function testAddExistentReturnsFalse()
    {
        $this->leadQueryServiceMock
            ->expects(self::once())
            ->method('getByEmail')
            ->with(self::EMAIL)
            ->willReturn(['email' => self::EMAIL]);

        $this->leadCommandHandlerMock
            ->expects(self::never())
            ->method('add');

        $result = $this->service->add(self::EMAIL);

        self::assertFalse($result);
    }

    public function testCanGetAll()
    {
        $unsorted = [
            ['email' => '[email protected]'],
            ['email' => '[email protected]'],
            ['email' => '[email protected]'],
        ];

        $this->leadQueryServiceMock
            ->expects(self::once())
            ->method('getAll')
            ->willReturn($unsorted);

        $fetched = $this->service->getAll();

        $expected = $unsorted;
        asort($expected);

        self::assertEquals($expected, $fetched);
    }
}

Integration tests can be written for infrastructure code. For instance, we can use an SQLite database file to assert the logic for database operations.

Be aware that I am creating an SQLite temp database file on-demand for each test execution with $this->databaseFilePath = '/tmp/test-' . time(); and, thanks to the Dbal library, we can be confident that operations could work for any database.

-> It is highly recommended that, as an alternative, create a container with a seeded database that is compatible with your production database system.

<?php

namespace Test\Integration\Blog\Infrastructure;

use Blog\Infrastructure\DbalLeadQueryService;
use Doctrine\DBAL\Connection;
use Faker\Factory;
use Faker\Generator;
use Test\TestCase;

class DbalLeadQueryServiceTest extends TestCase
{
    private string $databaseFilePath;

    private Generator $faker;
    private Connection $connection;

    private DbalLeadQueryService $service;

    public function testCanGetAll()
    {
        $this->addEmail($email1 = $this->faker->email());
        $this->addEmail($email2 = $this->faker->email());
        $this->addEmail($email3 = $this->faker->email());

        $expected = [
            ['email' => $email1],
            ['email' => $email2],
            ['email' => $email3],
        ];

        $fetched = $this->service->getAll();

        self::assertEquals($expected, $fetched);
    }

    protected function setUp(): void
    {
        parent::setUp();

        $this->faker = Factory::create();

        $this->createLeadTable();

        $this->service = new DbalLeadQueryService($this->connection());
    }

    protected function tearDown(): void
    {
        parent::tearDown();

        $this->dropDatabase();
    }

    private function connection(): Connection
    {
        if (!isset($this->connection)) {
            $this->databaseFilePath = '/tmp/test-' . time();

            $config = new \Doctrine\DBAL\Configuration();
            $connectionParams = [
                'url' => "sqlite:///{$this->databaseFilePath}",
            ];

            $this->connection = DriverManager::getConnection($connectionParams, $config);
        }

        return $this->connection;
    }

    private function dropDatabase()
    {
        @unlink($this->databaseFilePath);
    }

    private function createLeadTable(): void
    {
        $this->connection()->executeQuery('CREATE TABLE IF NOT EXISTS leads ( email VARCHAR )');
    }

    private function addEmail(string $email): int
    {
        return $this->connection()->insert('leads', ['email' => $email]);
    }
}

Conclusion

PHP is evolving very quickly, with new features that enable more quality software, help developers and is even more committed to the fact that most of the web run flavours of PHP code. New features improve readability, software architecture, test coverage and performance. Those are all proof of a mature and live language.

Upgrade to PHP 8.1 and use "new in initializers" as soon as possible. You will not regret it.

If there is something you want to discuss more, let me know in the comments.

Links:

  1. New in initializers RFC
  2. Road to PHP 8.1
  3. Dependency Injection
  4. Dependency inversion principle
  5. Interface segregation principle
  6. Solid relevance
  7. SOLID principles