wiremock-testcontainers-java

wiremock-testcontainers-java copied to clipboard

Motivation

• Param id does not bring any benefits and looks redundant
• Store all extensionJars in a LinkedHashSet to preserve an order and keep only unique values
• Store all extensionClassNames in a LinkedHashSet to preserve an order and keep only unique value

Thanks for the patch, but I actually did it for future extensibility and traceabilty reasons. Classes are private for the same reasons - internal use for now. I would like to keep the source ID in the metadata, and to extend that in the future. At the same time, it would be possible to introduce new methods that would infer the IDs from the file names by default, I guess it would address DevX simplicity case

I understand intention of this, but here is a few things that concerns me in current api

1. it forces developers to create random string literal for each method invocation, since this id has no influence on behavior values will be very random (absence of control / validation)

    new WireMockContainer()
        .withExtention("1", ..., ...)
        .withExtention("B", ..., ...)
        .withExtention("III", ..., ...)
   

   If maintainer of the library decide(in some future) to change meaning or utilization of id it will be painful.

2. Uniqueness of the extension = jarFile+className, so field id brings potential confusion

3. (assumption) In majority of case developers use one or two extensions - so there is will not be much things to trace

My suggestion about api is the following

withExtention(String className, File jar) // 1
withExtention(Collection<String> classNames, File jar) // 2
withExtention(Collection<String> classNames, Path directoryWithJars) // 3

But if in case of 1 & 2 this is easy to calculate unique identifier = className+jarName to keep in in metadata, then in case 3 it is sophisticated

My suggestion about api is the following

I agree with this approach. It is one of the ways to "infer" IDs like in the previous comment So I would add new methods that infer the IDs but would also keep the previous ones. No objections in making them package scope for now, but it would also mean a breaking change. No problem but maybe better to keep them

FTR my Slack comment:

Indeed it might be over-engineering at this stage. I am just looking at https://github.com/wiremock/wiremock/issues/1512 which @tomakehurst wants to see soon. It would be basically introducing a much more sophisticated engine, maybe with some notion for plugins and extension IDs like in Jenkins. So I put a very basic stub for the future, but indeed it might be conffusing as a standalone PR