CodelyTV / php-ddd-example

🐘🎯 Hexagonal Architecture + DDD + CQRS in PHP using Symfony 7
https://pro.codely.tv/library/ddd-en-php
2.96k stars 1.08k forks source link

Update test structure #30

Closed rgomezcasas closed 6 years ago

rgomezcasas commented 6 years ago

To a parallel test folder making it easier to find the test you're looking for

JavierCane commented 6 years ago

@xabi93 was just asking to me how to implement integration test in his PR: https://github.com/CodelyTV/cqrs-ddd-php-example/pull/29

One of the doubts was regarding the confusing folder structure, so maybe he's interested into this issue :P

Maybe we could define a test structure based on the one we've used for the https://github.com/CodelyTV/scala-http-api/ project:

$ tree scala-http-api/src/test/tv/codely/scala_http_api/module -L 5
.
├── IntegrationTestCase.scala
├── UnitTestCase.scala
├── shared
│   ├── domain <- Test infrastructure such as the application domain model stubs/object mother
│   │   ├── DateTimeStub.scala
│   │   ├── DurationStub.scala
│   │   ├── IntStub.scala
│   │   ├── StringStub.scala
│   │   └── UuidStub.scala
│   └── infrastructure <- Mix of test infrastructure and actual integration test for the infrastructure application layer implementations
│       ├── MessagePublisherMock.scala
│       ├── logger
│       │   └── scala_logging
│       │       └── ScalaLoggingLoggerShould.scala <- ScalaLoggingLogger class integration test
│       └── message_broker
│           └── rabbitmq
│               ├── MessageConsumer.scala <- Infrastructure needed for the integration test
│               ├── MessagePurger.scala <- Infrastructure needed for the integration test
│               ├── RabbitMqMessageConsumer.scala <- Infrastructure needed for the integration test
│               ├── RabbitMqMessagePublisherShould.scala <- RabbitMqMessagePublisher class integration test
│               └── RabbitMqMessagePurger.scala <- Infrastructure needed for the integration test
└── video
    ├── VideoIntegrationTestCase.scala
    ├── application
    │   ├── create
    │   │   └── VideoCreatorShould.scala <- Use case unit test
    │   └── search
    │       └── VideosSearcherShould.scala <- Use case unit test
    ├── domain <- Test infrastructure such as the application domain model stubs/object mother
    │   ├── SeqStub.scala
    │   ├── VideoCategoryStub.scala
    │   ├── VideoCreatedStub.scala
    │   ├── VideoDurationStub.scala
    │   ├── VideoIdStub.scala
    │   ├── VideoStub.scala
    │   └── VideoTitleStub.scala
    └── infrastructure <- Mix of test infrastructure and actual integration test for the infrastructure application layer implementations
        ├── marshaller
        │   └── VideoJsValueMarshaller.scala <- Test infrastructure needed in order to avoid using the production marshaller and actually testing its behaviour
        └── repository
            ├── DoobieMySqlVideoRepositoryShould.scala <- DoobieMySqlVideoRepository class integration test
            └── VideoRepositoryMock.scala <- Test infrastructure needed in order to do not use production implementations in unit tests

However, I'm not 100% confident about it. So I'm not sure if we would have to replicate it in this project, or iterate it a little bit. Here you have an alternative I would take into account before refactoring:

$ tree scala-http-api/src/test/tv/codely/scala_http_api/module -L 5
.
├── shared
│   ├── infrastructure
│   │   ├── IntegrationTestCase.scala
│   │   ├── UnitTestCase.scala
│   │   ├── message_broker
│   │   │   ├── MessagePublisherMock.scala
│   │   │   └── rabbitmq
│   │   │       ├── MessageConsumer.scala
│   │   │       ├── MessagePurger.scala
│   │   │       ├── RabbitMqMessageConsumer.scala
│   │   │       └── RabbitMqMessagePurger.scala
│   │   └── stub
│   │       ├── DateTimeStub.scala
│   │       ├── DurationStub.scala
│   │       ├── IntStub.scala
│   │       ├── StringStub.scala
│   │       └── UuidStub.scala
│   └── integration
│       ├── logger
│       │   └── ScalaLoggingLoggerShould.scala
│       └── message_broker
│           └── RabbitMqMessagePublisherShould.scala
└── video
    ├── infrastructure <- Only test infrastructure
    │   ├── VideoIntegrationTestCase.scala
    │   ├── marshaller
    │   │   └── VideoJsValueMarshaller.scala
    │   ├── repository
    │   │   └── VideoRepositoryMock.scala
    │   └── stub <- Stubs/Object mother also understood as test infrastructure
    │       ├── SeqStub.scala
    │       ├── VideoCategoryStub.scala
    │       ├── VideoCreatedStub.scala
    │       ├── VideoDurationStub.scala
    │       ├── VideoIdStub.scala
    │       ├── VideoStub.scala
    │       └── VideoTitleStub.scala
    ├── integration <- Only integration tests
    │   └── repository
    │       └── DoobieMySqlVideoRepositoryShould.scala
    └── unit <- Only unit tests
        ├── VideoCreatorShould.scala
        └── VideosSearcherShould.scala

We've discussed sometimes about this, and I'm neither 100% sure about this second approach, however, there are some pros that I really like despite the cons:

Pros:

Cons:

What do you think?

rgomezcasas commented 6 years ago

I prefer the first 😬

JavierCane commented 6 years ago

I don't have a really strong opinion here, so… go ahead with the first approach then 🙂

rgomezcasas commented 6 years ago

😬😬😬

x4b1 commented 6 years ago

I think having tests separately in another folder is cleaner, and making it easier to know where to add another test. Now I have to add integration tests for the telegram notification and I don't know where I have to modify, but maybe this is my fault, not because of the project structure. 😞

JavierCane commented 6 years ago

Not at all @xabi93!

We're evaluating this change because the current tests structure is not clear enough. So it's completely understandable how it can frustrate anyone new to the project facing tasks as the one you're trying to do.

In the meantime, I had posted a comment in your PR (https://github.com/CodelyTV/cqrs-ddd-php-example/pull/29#issuecomment-362893088) explaining how could you add this integration test in the current test structure. We'll move it with all the other test code when we take action on this issue, but this way you can continue with your PR without being blocked 🙂