Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
skrilagg opened this issue Dec 16, 2021 · 3 comments
Assignees

Comments

@skrilagg
Copy link

skrilagg commented Dec 16, 2021

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:

  • When creating deepsampler tests I start with a SaveSamples-Test with the name of the service method for example.
  • After that I create another LoadSamples-Test which and comment out the Test Annotation on the LoadSamples method.
  • The default behaviour now would be that I have to either give both methods the same name or set the source name on the LoadSamples-Test everytime.

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
Copy link
Contributor

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:

@SaveSamples(packagePath = SAVED_IN_SPECIFIC_PACKAGE, fileName = SAVED_IN_SPECIFIC_FILE)
@Order(3)
void whenSamplerIsSavedInSpecificPackageAndFile() throws IOException {

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.

@JanSchankin JanSchankin self-assigned this Dec 17, 2021
@skrilagg
Copy link
Author

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants