microcks / microcks-testcontainers-go-demo

Go demonstration app on how to use Microcks Testcontainers in your dev/test workflow
https://microcks.io
MIT License
5 stars 2 forks source link

TestOpenAPIContractOld name could be missleading #4

Closed oliviernguyenquoc closed 1 month ago

oliviernguyenquoc commented 2 months ago

Describe the bug

In https://github.com/microcks/microcks-testcontainers-go-demo/blob/79d05a5f1c15e93f294d845deb60e5589fb173ad/internal/controller/order_controller_test.go#L52

In opposition to TestOpenAPIContract, TestOpenAPIContractOldis using this setup method:

microcksContainer, err := microcks.RunContainer(ctx,
        testcontainers.WithImage("quay.io/microcks/microcks-uber:1.9.1-native"),
        microcks.WithMainArtifact("../../testdata/order-service-openapi.yaml"),
        microcks.WithMainArtifact("../../testdata/apipastries-openapi.yaml"),
        microcks.WithSecondaryArtifact("../../testdata/apipastries-postman-collection.json"),
        microcks.WithHostAccessPorts([]int{server.DefaultApplicationPort}),
    )

instead of:

microcksEnsemble, err := ensemble.RunContainers(ctx,
        ensemble.WithMainArtifact("../../testdata/order-service-openapi.yaml"),
        ensemble.WithMainArtifact("../../testdata/apipastries-openapi.yaml"),
        ensemble.WithSecondaryArtifact("../../testdata/apipastries-postman-collection.json"),
        ensemble.WithPostman(true),
        //ensemble.WithDefaultNetwork(),
        ensemble.WithHostAccessPorts([]int{server.DefaultApplicationPort}),
    )

TestOpenAPIContractOld name could indicated that this way of writing it is deprecated. I think this setup is prefered in case of non-usage use of "Advanced features".

What about TestOpenAPIContractBasicUsage and TestOpenAPIContractAdvanceUsage ?

Or maybe I'm wrong and ensemble setup is the way to go for all use cases.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

No response

Microcks version or git rev

No response

Install method (docker-compose, helm chart, operator, docker-desktop extension,...)

No response

Additional information

No response

lbroudoux commented 2 months ago

You're totally right. Advanced and Basic are much more well suited than Old. As our Go testcontainers is still a WIP (waiting for these network enhancements on Mac), I think we have to go through some sanitization and review before next release!

Marking it as enhancement for the next batch

lbroudoux commented 1 month ago

Hey there!

I had a review and in fact as we are now using the cmd/run/run.go to dynamically start a full application, we need to provide a full application configuration including kafka server connection - otherwise the application won't start. So as of now, it's no longer possible to use the TestOpenAPIContfactBasic without creating a full ensemble.

I think I'll gonna drop this to avoid confusion in next release.

lbroudoux commented 1 month ago

Now fixed!