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.

Sign up for GitHub

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

Pass exceptions on to manufacture method #107

Merged
merged 4 commits into from
Aug 25, 2024
Merged

Pass exceptions on to manufacture method #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
97f6111
feat: pass exceptions on to manufacture
Nylle Aug 22, 2024
736d4a2
test: assert cause
jk-idealo Aug 23, 2024
f0cc7f2
test: increase test-coverage
Nylle Aug 25, 2024
8f70cd9
fix: flaky test
akutschera Aug 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/main/java/com/github/nylle/javafixture/InstanceFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,27 +57,27 @@ public InstanceFactory(SpecimenFactory specimenFactory) {
this.random = new PseudoRandom();
}

public <T> T construct(final SpecimenType<T> type, CustomizationContext customizationContext) {
public <T> T construct(SpecimenType<T> type, CustomizationContext customizationContext) {
return random.shuffled(type.getDeclaredConstructors())
.stream()
.filter(x -> Modifier.isPublic(x.getModifiers()))
.findFirst()
.map(x -> construct(type, x, customizationContext))
.orElseGet(() -> manufacture(type, customizationContext));
.orElseGet(() -> manufacture(type, customizationContext, new SpecimenException("No public constructor found")));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add the name of the class for which no public constructor was found. It could be that this is not the class I am fixturing but one of its fields.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if we add something like "trying factory methods next" to the message, it would improve the understandability of the stack trace.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should not do exceptions at all and implement proper strategies?

I mean, this can certainly be improved. But do you think this is mergeable as a first attempt with improving it on further iterations?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long comment short: Yes, I think we should try it.
Let's label it as experimental because it will only affect diagnosis, not behaviour.

}

public <T> T manufacture(final SpecimenType<T> type, CustomizationContext customizationContext) {
public <T> T manufacture(SpecimenType<T> type, CustomizationContext customizationContext, Throwable throwable) {
return random.shuffled(type.getFactoryMethods())
.stream()
.filter(method -> Modifier.isPublic(method.getModifiers()))
.filter(method -> !hasSpecimenTypeAsParameter(method, type))
.map(x -> manufactureOrNull(x, type, customizationContext))
.filter(x -> x != null)
.findFirst()
.orElseThrow(() -> new SpecimenException(format("Cannot create instance of %s", type.asClass())));
.orElseThrow(() -> new SpecimenException(format("Cannot create instance of %s: no factory-method found", type.asClass()), throwable));
}

public <T> T instantiate(final SpecimenType<T> type) {
public <T> T instantiate(SpecimenType<T> type) {
try {
return type.asClass().getDeclaredConstructor().newInstance();
} catch (Exception e) {
Expand Down Expand Up @@ -111,8 +111,8 @@ private <T> T construct(final SpecimenType<T> type, final Constructor<?> constru
return (T) constructor.newInstance(stream(constructor.getParameters())
.map(p -> createParameter(p, customizationContext))
.toArray());
} catch (Exception e) {
return manufacture(type, customizationContext);
} catch (Exception ex) {
return manufacture(type, customizationContext, ex);
}
}

Expand Down Expand Up @@ -149,8 +149,8 @@ private <T> T createProxyForAbstract(final SpecimenType<T> type, final Map<Strin
var factory = new ProxyFactory();
factory.setSuperclass(type.asClass());
return (T) factory.create(new Class<?>[0], new Object[0], new ProxyInvocationHandler(specimenFactory, specimens));
} catch (Exception e) {
throw new SpecimenException(format("Unable to create instance of abstract class %s: %s", type.asClass(), e.getMessage()), e);
} catch (Exception ex) {
throw new SpecimenException(format("Unable to create instance of abstract %s: %s", type.asClass(), ex.getMessage()), ex);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/github/nylle/javafixture/specimen/AbstractSpecimen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ public T create(final CustomizationContext customizationContext, Annotation[] an
field.getGenericType().getTypeName(),
specimenFactory.build(SpecimenType.fromClass(field.getGenericType()))).create(new CustomizationContext(List.of(), Map.of(), false), field.getAnnotations()))));
return context.remove(type);
} catch(SpecimenException ignored) {
return instanceFactory.manufacture(type, customizationContext);
} catch(SpecimenException ex) {
return instanceFactory.manufacture(type, customizationContext, ex);
}
}
}
Expand Down
Loading
Loading