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

Exception thrown in record compact constructor causes test failure #613

Closed
C-Otto opened this issue Mar 19, 2022 · 10 comments · Fixed by #614
Closed

Exception thrown in record compact constructor causes test failure #613

C-Otto opened this issue Mar 19, 2022 · 10 comments · Fixed by #614
Labels

Comments

@C-Otto
Copy link

C-Otto commented Mar 19, 2022

Describe the bug
Exceptions thrown in the compact constructor of a record causes EqualsVerifier to fail the test.

To Reproduce

public record Foo(String data) {
    public Foo {
        if (data.length() <= 3) {
            throw new IllegalStateException(data);
        }
    }
}

In my test: EqualsVerifier.forClass(Foo.class).suppress(Warning.NULL_FIELDS).verify();

The compact constructor is the result of converting a regular Java class:

public Foo(String data) {
    if (data.length() <= 3) {
        throw new IllegalStateException(data);
    }
    this.data = data;
}

(private final field, getter, generated equals/hashCode)

Error message

EqualsVerifier found a problem in class somewhere.Foo.
-> Record: failed to invoke constructor.

For more information, go to: https://www.jqno.nl/equalsverifier/errormessages
java.lang.AssertionError: EqualsVerifier found a problem in class somewhere.Foo.
-> Record: failed to invoke constructor.

For more information, go to: https://www.jqno.nl/equalsverifier/errormessages
	at nl.jqno.equalsverifier.api.SingleTypeEqualsVerifierApi.verify(SingleTypeEqualsVerifierApi.java:316)
	at somewhere.FooTest.name(FooTest.java:10)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725)
	at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:140)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:84)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
	at org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
	at org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$7(TestMethodTestDescriptor.java:214)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:210)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135)
	at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:66)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
	at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
	at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
	at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
	at org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
	at org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:108)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
	at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:96)
	at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:99)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:79)
	at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:75)
	at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:61)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
	at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
	at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
	at jdk.proxy2/jdk.proxy2.$Proxy5.stop(Unknown Source)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
	at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
	at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:133)
	at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
	at worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Caused by: nl.jqno.equalsverifier.internal.exceptions.ReflectionException: Record: failed to invoke constructor.
	at app//nl.jqno.equalsverifier.internal.util.Rethrow.rethrow(Rethrow.java:34)
	at app//nl.jqno.equalsverifier.internal.reflection.RecordObjectAccessor.callRecordConstructor(RecordObjectAccessor.java:140)
	at app//nl.jqno.equalsverifier.internal.reflection.RecordObjectAccessor.makeAccessor(RecordObjectAccessor.java:119)
	at app//nl.jqno.equalsverifier.internal.reflection.RecordObjectAccessor.scramble(RecordObjectAccessor.java:61)
	at app//nl.jqno.equalsverifier.internal.reflection.ClassAccessor.getRedAccessor(ClassAccessor.java:179)
	at app//nl.jqno.equalsverifier.internal.reflection.ClassAccessor.getRedObject(ClassAccessor.java:168)
	at app//nl.jqno.equalsverifier.internal.util.Configuration.ensureUnequalExamples(Configuration.java:199)
	at app//nl.jqno.equalsverifier.internal.util.Configuration.build(Configuration.java:99)
	at app//nl.jqno.equalsverifier.api.SingleTypeEqualsVerifierApi.buildConfig(SingleTypeEqualsVerifierApi.java:381)
	at app//nl.jqno.equalsverifier.api.SingleTypeEqualsVerifierApi.performVerification(SingleTypeEqualsVerifierApi.java:367)
	at app//nl.jqno.equalsverifier.api.SingleTypeEqualsVerifierApi.verify(SingleTypeEqualsVerifierApi.java:312)
	... 84 more
Caused by: java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at nl.jqno.equalsverifier.internal.reflection.RecordObjectAccessor.lambda$callRecordConstructor$4(RecordObjectAccessor.java:141)
	at nl.jqno.equalsverifier.internal.util.Rethrow.rethrow(Rethrow.java:30)
	... 94 more
Caused by: java.lang.IllegalStateException: one
	at somewhere.Foo.<init>(Foo.java:6)
	... 101 more

Expected behavior
I expect EqualsVerifier to pass the test. In general, I expect the same behaviour for classes and equivalent records.

Version
3.9.1

Additional context
Java 17

@C-Otto
Copy link
Author

C-Otto commented Mar 20, 2022

Note that this is also an issue when testing a class which uses a record as a field (which needs to be instantiated by EqualsVerifier). Here, using .withPrefabValues is a suitable workaround.

@jqno
Copy link
Owner

jqno commented Mar 21, 2022

Unfortunately, records in Java are well protected from reflection. (Actually, that's a good thing, but it makes things for EqualsVerifier a little more difficult.) We can instantiate classes via reflection without calling the constructor, but for records, the constructor must be called, no exceptions.

So at the very least, if you want to check invariants like you do in your example, we should add some prefab values, like this:

EqualsVerifier.forClass(Foo.class)
        .withPrefabValues(int.class, 3, 4)
        .verify();

Unfortunately, EqualsVerifier still has some tests where it plugs default values (i.e., 0) into fields, so it will become something more like this:

EqualsVerifier.forClass(Foo.class)
        .withPrefabValues(int.class, 3, 4)
        .suppress(Warning.DEFAULT_VALUES)
        .verify();

Where Warning.DEFAULT_VALUES doesn't exist yet; I'll have to implement that. (I might also pick a better name if I can think of one.)

I agree that this is a desirable thing for EqualsVerifier, so I will try to impement it soon, if I can find some spare time. But it will never look as nice as when using regular classes, unfortunately.

@jqno jqno added the accepted label Mar 21, 2022
@C-Otto
Copy link
Author

C-Otto commented Mar 21, 2022

Thanks! It might suffice to change the error messages for exceptions thrown by some record constructor, and hint at the prefab values trick. I'm quite happy with this workaround, as I already have test fixtures. Plugging in .withPrefabValues(Foo.class, FOO, FOO_2) isn't that bad.

@jqno
Copy link
Owner

jqno commented Mar 21, 2022

I think I have a solution in PR #614 . Your test would then look like this:

EqualsVerifier
.forClass(ValueCheckingRecord.class)
.withPrefabValues(int.class, 10, 11)
.suppress(Warning.ZERO_FIELDS)
.verify();

I can't merge the PR yet because I need to write some documentation for it.

@jqno jqno closed this as completed in #614 Mar 22, 2022
@jqno
Copy link
Owner

jqno commented Mar 22, 2022

I've just released version 3.10 with this fix. There's also some documentation at https://jqno.nl/equalsverifier/errormessages/record-failed-to-invoke-constructor/

@lukeu
Copy link

lukeu commented Jun 28, 2022

A compact-constructor asserting non-null objects also triggers this.

However I was initially quite confused by the (updated) error message & advice, as I could not find any numeric zeros around. (I think also "Record: failed to invoke constructor" gave the impression of some kind of internal limitation before invoking the constructor - it was successfully invoked, it just didn't complete.)

However after a (rubbish) night's sleep, I've now joined the dots.

I imagine use of Objects.requireNonNull must be a common use case? e.g.:

record Example(String name, @Nullable String profession) {
    public Example {
        Objects.requireNonNull(name);
    }

Should the error message be extended to include null, and the documentation updated with a further remedy?

    EqualsVerifier.forClass(Example.class)
            .withNonnullFields("name")
            .verify();

Hopefully these 2 examples would then be enough to help the developer extrapolate any further edge cases.

Although I guess even better if it could detect NPE (which requireNonNull and Guava's Preconditions.checkNotNull both throw) and offer a more specific message.

@jqno
Copy link
Owner

jqno commented Jun 29, 2022

That's a good suggestion, I'll reopen this ticket and pick it up for a next version.

I can't promise a timeframe though, as I don't have a lot of free time to work on EqualsVerifier these days.

@jqno
Copy link
Owner

jqno commented Jul 26, 2022

I've just released version 3.10.1 which has better error reporting for exceptions in record constructors.

@jqno jqno closed this as completed Jul 26, 2022
@lukeu
Copy link

lukeu commented Aug 21, 2022

Hi, I've just returned from extended holiday & taken a glance. New documentation page looks excellent, nicely done!

I noticed the link added to the list of error messages is broken, though. (Hopefully you'll have seen my comment in the source. It was my first code-review style comment via github.)

@jqno
Copy link
Owner

jqno commented Aug 22, 2022

Thanks for noticing, I've fixed the link!

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

Successfully merging a pull request may close this issue.

3 participants