Giter VIP home page Giter VIP logo

Comments (12)

HellPat avatar HellPat commented on August 16, 2024 1

You're welcome. Let me know if you have further questions an I'll share my opinion

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

Our current basic test:

final class LoginControllerTest extends AbstractControllerTest
{
    /** @test */
    public function itCanBeAccessedAnonymously(): void
    {
        $commandHandlerMock = $this->getMockBuilder(LoginHostHandler::class)
            ->disableOriginalConstructor()
            ->getMock();

        $this->client->getContainer()->set(LoginHostHandler::class, $commandHandlerMock);

        $content = [
            'emailAddress' => '[email protected]',
            'password' => 'test1234'
        ];

        $this->client->request('POST', '/login', [], [], [], json_encode($content));

        $this->assertTrue($this->client->getResponse()->isSuccessful());
    }
}

from php-ddd.

HellPat avatar HellPat commented on August 16, 2024

Seems that your example is not correct, HostReadModel is never beeing used?
If you want to test "service command handlers dealing with read models" mocking them seems somehow wrong?

IMO you can use your real LoginHostHandler and mock away every dependency of ``LoginHostHandler` which have side effects?

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

Seems that your example is not correct, HostReadModel is never beeing used?

Thank you! The $hostDetails is the read model. I added a @var doc to it.

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

IMO you can use your real LoginHostHandler and mock away every dependency of ``LoginHostHandler` which have side effects?

I must confess I did not understand this line. Could you please explain?

from php-ddd.

HellPat avatar HellPat commented on August 16, 2024

The $hostDetails is the read model. I added a @var doc to it.

ah okay, thanks :-)

Your subject under test is your LoginHostHandler (as stated in the ticket title).
And in your example test you mock exactly this LoginHostHandler

$commandHandlerMock = $this->getMockBuilder(LoginHostHandler::class)
            ->disableOriginalConstructor()
            ->getMock();

Then you overwrite the LoginHostHandler with your mock:

$this->client->getContainer()->set(LoginHostHandler::class, $commandHandlerMock);

then you call your Application (via http I guess)

$this->client->request('POST', '/login', [], [], [], json_encode($content));

So the question is: why don't you just test the handler it self (as stated in the issue title)?

You could do this, while abstracting the readModel away:

function it_errors_on_unverified_host()
{
    $alwaysUnverified = new class implements HostDetailsFinder {
        public function isVerified() { return false; }
    }

    $subjectUnderTest = new LoginHostHandler(
        $alwaysUnverified,
        new UserPasswordEncoder
        new JWTEncoder
    );

    $this->expectException(HostNotVerifiedException::class);

    $subjectUnderTest(new LoginHost([
          'emailAddress' => '[email protected]',
          'password' => 'test1234'
    ]));
}

What did we do:

  1. we unit-tested LoginHostHandler in isolation, without framework
  2. we mocked away the readmodel, since it's not "subject under test"
  3. since readmodel is mocked you can test multiple scenarios

If you want to functional test your app you can mock the readmodel away the same way, but for a functional test I'd prefer to produce the data you need and test your application end2end.

Regarding your questions:

First of all, would you move the logic away from the handler e.g. to a "LoginService"?

no would't.
After all your Handler is already a single function service - reusable - totally fine.

Would you even consider loading the WRITE model (event-sourced aggregate-root) though you don't want to change state?

only if i have to read state from the aggregate root without eventual concistency.
Since you already have a ReadModel I'd use that.

Would you e.g. mock the repository or type-hint the interface and test different read model scenarios. Keeping the logic inside the command handler?

Im not 100% sure what you mean here, but when testing in isolation it's very easy to test multiple scenarios based on the ReadModel.

Hope I understood your questions :-)

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

Sorry for the late reply @HellPat !

So the question is: why don't you just test the handler it self (as stated in the issue title)?

I must confess the featured test file was confusing. The controller actually was only used to check if routes are correctly protected by the authentication.
The handlers aren't really part of this test. And mocking was the easiest way to get through to the thrown http status.

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

First of all, would you move the logic away from the handler e.g. to a "LoginService"?

no would't.
After all your Handler is already a single function service - reusable - totally fine.

That's good news.

    $alwaysUnverified = new class implements HostDetailsFinder {
        public function isVerified() { return false; }
    }

That is a very helpful part! I think that was the missing link.

I will give it a try! Thank you very much.

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024

BTW @HellPat Are you on Twitter?

from php-ddd.

HellPat avatar HellPat commented on August 16, 2024

Yep, this is how I found your Issue :-)

https://twitter.com/psren

from php-ddd.

webdevilopers avatar webdevilopers commented on August 16, 2024 
final class UniqueValueAddedTaxIdentifierNumberPolicy
{
    private HostReadModelRepository $repository;

    public function __construct(HostReadModelRepository $repository)
    {
        $this->repository = $repository;
    }

    public function isSatisfiedBy(HostId $hostId, ValueAddedTaxIdentificationNumber $vatId): bool
    {
        try {
            $host = $this->repository->ofVatId($vatId);
        } catch (HostNotFoundException $exception) {
            return true;
        }

        if ($host->hostId()->sameValueAs($hostId)) {
            return true;
        }

        return false;
    }
}
interface HostReadModelRepository
{
    public function ofHostId(HostId $hostId): HostReadModel;

    public function ofEmailAddress(EmailAddress $emailAddress): HostReadModel;

    public function ofVerificationToken(string $token): HostReadModel;

    public function ofVatId(ValueAddedTaxIdentificationNumber $vatId): HostReadModel;
}
final class UniqueValueAddedTaxIdentifierNumberPolicyTest extends TestCase
{
    /** @test */
    public function itIsSatisfiedWhenVatIdIsUsedBySameHost(): void
    {
        $hostReadModelRepository = new class implements HostReadModelRepository {
            public function ofHostId(HostId $hostId): HostReadModel
            {}

            public function ofEmailAddress(EmailAddress $emailAddress): HostReadModel
            {}

            public function ofVerificationToken(string $token): HostReadModel
            {}

            public function ofVatId(ValueAddedTaxIdentificationNumber $vatId): HostReadModel
            {
                return HostReadModel::fromArray([
                    'hostId' => 'b38f676e-221d-4807-97c9-8f549b13425e',
                    'emailAddress' => '[email protected]',
                    'verified' => true,
                    'profileCompleted' => true,
                    'activePlaces' => false,
                    'activePlan' => false,
                ]);
            }
        };

        $policy = new UniqueValueAddedTaxIdentifierNumberPolicy($hostReadModelRepository);

        $this->assertTrue($policy->isSatisfiedBy(
            HostId::fromString('b38f676e-221d-4807-97c9-8f549b13425e'),
            ValueAddedTaxIdentificationNumber::fromString('DE 123456789')
        ));
    }
}

Hi @HellPat , here is a different unit test that also involves repositories.
Following your example I recognized that anonymous classes do not have to implement the body or return value of the interface.

    public function ofHostId(HostId $hostId): HostReadModel;

Can be written...

new class implements HostReadModelRepository {
            public function ofHostId(HostId $hostId): HostReadModel
            {}

...without any return value at all.

Though PHPStorm warns me about it:

Screenshot from 2020-12-04 15-56-06

Should I change this and / or are there any more ways to shorten the code for the test?

Thanks in advance.

from php-ddd.

HellPat avatar HellPat commented on August 16, 2024

Yeah,
If you have an Interface you MUST fulfill it. The more methods you have, the more complex it gets.

In general I'd not mock the repository.
The repository is interacting with your database, and if you test your Repository you should test it against the database.

e.g.

  • perform some actions (commands)
  • look if these actions lead to the desired result (read: the repository returns the right things)

If you want to test UniqueValueAddedTaxIdentifierNumberPolicy you could pass the Readmodel directly.

e.g.

final class UniqueValueAddedTaxIdentifierNumberPolicy
{
    public function isSatisfiedBy(HostId $hostId, HostReadModel $host): bool
    {
        return $host->hostId()->sameValueAs($hostId);
    }
}

You can catch HostNotFoundException at the place you use the Repository (where UniqueValueAddedTaxIdentifierNumberPolicy is beeing used, or in a global ExceptionHandler)

Your looks like

final class UniqueValueAddedTaxIdentifierNumberPolicyTest extends TestCase
{
    /** @test */
    public function itIsSatisfiedWhenVatIdIsUsedBySameHost(): void
    {
        $hostReadModel = HostReadModel::fromArray([
               'hostId' => 'b38f676e-221d-4807-97c9-8f549b13425e',
                'emailAddress' => '[email protected]',
                'verified' => true,
                'profileCompleted' => true,
                'activePlaces' => false,
                'activePlan' => false,
            ]);

        $policy = new UniqueValueAddedTaxIdentifierNumberPolicy();

        $this->assertTrue($policy->isSatisfiedBy(
            HostId::fromString('b38f676e-221d-4807-97c9-8f549b13425e'),
            $hostReadModel
        ));
    }
}

but to be honest, you should already have a test for sameValueAs so the class UniqueValueAddedTaxIdentifierNumberPolicy is not necessary at all.

I'm not entirely sure what you want to test.

from php-ddd.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.