ppi-ag / deep-sampler

DeepSampler is a stubbing framework for compound tests
https://ppi.de
MIT License
11 stars 2 forks source link

Enhancement: Add support to configure if the method suffix should be used for created JSON file name #96

Open skrilagg opened 2 years ago

skrilagg commented 2 years ago

Hi all,

I am using version 1.1.0 and therefore had to add some functionality, that in parts is already added for version 2.x.

Part of this is the ability to configure a base repository for stored files, which I just noticed you already implemented for the next version. Another addition I added is the ability to configure if the name of the testMethod should be used in the generated json file name.

Let me explain, why I added this:

For convenience I added another configuration value to the Annotation UseSamplerFixture, which is then used inside the JUnitPluginUtils. Let me show you with the following:

public static void saveSamples(final Method testMethod) {
    final SaveSamples saveSamples = testMethod.getAnnotation(SaveSamples.class);

    if (saveSamples == null) {
        return;
    }

    final JsonSourceManager.Builder persistentSampleManagerBuilder = loadBuilder(
            saveSamples.persistenceManagerProvider());

    final String fileName = getFilePath(saveSamples, getFixture(testMethod), testMethod);

    PersistentSamplerHolder.source(persistentSampleManagerBuilder.buildWithFile(fileName));

    addExtensions(testMethod);

    PersistentSamplerHolder.getSampler().record();
}

private static String getFilePath(LoadSamples loadSamples, UseSamplerFixture fixture, Method testMethod) {
    return getFilePath(loadSamples.file(), fixture, testMethod);
}

private static String getFilePath(SaveSamples saveSamples, UseSamplerFixture fixture, Method testMethod) {
    return getFilePath(saveSamples.file(), fixture, testMethod);
}

private static String getFilePath(String path, UseSamplerFixture fixture, Method testMethod) {
    String basePath = fixture != null ? fixture.repositoryBase() : "";
    String file = StringUtils.defaultIfEmpty(path,
            getDefaultJsonFileNameWithFolder(testMethod, withMethod(fixture)));

    return Paths.get(basePath, file).toString();
}

private static boolean withMethod(final UseSamplerFixture fixture) {
    return Optional.ofNullable(fixture).map(anno -> anno.withMethod()).orElse(false);
}

private static String getDefaultJsonFileNameWithFolder(final Method testMethod, boolean withMethod) {
    return testMethod.getDeclaringClass().getName().replace(".", "/")
            + (withMethod ? ("_" + testMethod.getName()) : "") + ".json";
}

Let me hear your thoughts on this.

Cheers, Michel

JanSchankin commented 2 years ago

Hi Michel,

yes, we introduce the annotation @SampleRootPath in 2.0.0. The annotation can be used on the test class or on the SamplerFixture. It can be used to define a base path of a central test repository.

We have had an intermediate version where the complete path of a sample file was composed of three parts, like this:

[root path][package or folder][file name]

Each part has a default value and can be configured manually. The default is generated using the package of the test class and the name of the test method, exactly as it was done in 1.1.0. I think this might be what you are looking for:

[manually configured root path][default: generated folder using the package of the test class][manually defined file name]

I wasn't happy with this, because the API got a little hard to understand, and I wasn't sure if this really would be used. So I decided to refactor this to a version with only two parts:

[root path][path to file]

Maybe we should consider to switch back to the three-part version. You can find an example for that here:

https://github.com/ppi-ag/deep-sampler/blob/f7f1d5ad9e25f08dd3366bb2471f9aff123df3b4/deepsampler-junit5/src/test/java/de/ppi/deepsampler/junit5/PersistentSamplerTest.java#L69-L71

This is part of the branch

https://github.com/ppi-ag/deep-sampler/tree/feature_configurable_json_path_3_parts

Maybe we can find a design that offers both, a simple configuration and an "advanced" configuration. What do you think?

BTW: Since you are already coding in DeepSampler, you are welcome to fork the repo and create pull requests, if you like.

skrilagg commented 2 years ago

Hi Jan,

I will give it a shot once I got the project working. Gradle KTS seems a little tricky when using Eclipse, but I'll sort that out.

JanSchankin commented 2 years ago

Yes, that might be easier if you have the chance to use Idea. Even with the community edition gradle works out of the box.