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

misleading message when SpringFactory doesn't start correctly #2584

Closed
vincent-fuchs opened this issue Jul 7, 2022 · 5 comments · Fixed by #2585
Closed

misleading message when SpringFactory doesn't start correctly #2584

vincent-fuchs opened this issue Jul 7, 2022 · 5 comments · Fixed by #2585

Comments

@vincent-fuchs
Copy link

👓 What did you see?

Using Cucumber BOM 7.4.1 on a Spring Boot 2.7 project, I am launching my cucumber test. I see the Spring Context loading, but then the test fails and I get a long stacktrace :

io.cucumber.core.backend.CucumberBackendException: No test instance
	at app//io.cucumber.spring.TestContextAdaptor.notifyTestContextManagerAboutAfterTestMethod(TestContextAdaptor.java:129)
	at app//io.cucumber.spring.TestContextAdaptor.stop(TestContextAdaptor.java:109)
	at app//io.cucumber.spring.SpringFactory.stop(SpringFactory.java:159)
	at app//io.cucumber.core.runner.Runner.disposeBackendWorlds(Runner.java:156)
	at app//io.cucumber.core.runner.Runner.runPickle(Runner.java:78)
	at app//io.cucumber.junit.platform.engine.CucumberEngineExecutionContext.lambda$runTestCase$4(CucumberEngineExecutionContext.java:112)
	at app//io.cucumber.core.runtime.CucumberExecutionContext.lambda$runTestCase$5(CucumberExecutionContext.java:129)

After investigating in debug mode, I realized my scenario is found, and I am entering in the glue-code, but failing immediately :

  public DwhStepDef(TestRestTemplate restTemplate, DwhDao dwhDao) {

    DataTableType(
        (Map<String, String> row) -> Dwh.builder().code(Long.parseLong(row.get("code")))
            .description(row.get("description")).build());
    DataTableType(
        (Map<String, String> row) -> DwhEntity.builder().code(Long.parseLong(row.get("code")))
            .description(row.get("description"))
            .build());
.....

using DataTableType here actually throws an InvocationTargetException :

java.lang.NullPointerException: Cannot invoke "io.cucumber.java8.LambdaGlueRegistry.addDataTableType(io.cucumber.core.backend.DataTableTypeDefinition)" because the return value of "java.lang.ThreadLocal.get()" is null

This code used to work with previous versions, and that's fine, I will change it.

But the message we are getting by default is really misleading (and I just spent 2h to understand what was wrong)

✅ What did you expect to see?

The exception that is thrown should be seen in the logs, but it's not : the system is in an inconsistent state, and this is caught at the end, providing an error message that is totally unrelated with the actual issue.

📦 Which tool/library version are you using?

in io.cucumber.core.runner.Runner 7.4.1 , I see :

public void runPickle(Pickle pickle) {
    try {
        StepTypeRegistry stepTypeRegistry = createTypeRegistryForPickle(pickle);
        snippetGenerators = createSnippetGeneratorsForPickle(stepTypeRegistry);

        // Java8 step definitions will be added to the glue here
        buildBackendWorlds();

        glue.prepareGlue(stepTypeRegistry);

        TestCase testCase = createTestCaseForPickle(pickle);
        testCase.run(bus);
    } finally {
        glue.removeScenarioScopedGlue();
        disposeBackendWorlds();
    }
}

The exception is thrown by the call to buildBackendWorlds . Is it normal that there's no catch block and that we directly go to the finally block ? I would say that having a catch block would enable to log the original exception, and lead the developer directly in the right direction.

🔬 How could we reproduce it?

I guess with any glue code that throws some Exception at instantiation time..

📚 Any additional context?

with v7.3.4 or v7.2.3 , my test also fails, but I get a proper error message that tells me what is wrong :

Caused by: org.springframework.beans.BeanInstantiationException: Failed to instantiate [my.project.dwh.cucumber.DwhStepDef]: Constructor threw exception; nested exception is java.lang.NullPointerException: Cannot invoke "io.cucumber.java8.LambdaGlueRegistry.addBeforeHookDefinition(io.cucumber.core.backend.HookDefinition)" because the return value of "java.lang.ThreadLocal.get()" is null
at app//org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:224)
at app//org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:117)
at app//org.springframework.beans.factory.support.ConstructorResolver.instantiate(ConstructorResolver.java:311)
... 105 more
Caused by: java.lang.NullPointerException: Cannot invoke "io.cucumber.java8.LambdaGlueRegistry.addBeforeHookDefinition(io.cucumber.core.backend.HookDefinition)" because the return value of "java.lang.ThreadLocal.get()" is null
at io.cucumber.java8.LambdaGlue.Before(LambdaGlue.java:69)
at my.project.dwh.cucumber.DwhStepDef.(DwhStepDef.java:45)
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 org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:211)
... 107 more

@mpkorstanje
Copy link
Contributor

This code used to work with previous versions, and that's fine, I will change it.

It doesn't look like you're doing anything wrong. Would I have to anything aside from your example code to reproduce the problem?

Is it normal that there's no catch block and that we directly go to the finally block?

Yes. Provided disposeBackendWorlds does not throw anything the original exception will be propagated. But Cucumber and Spring have an impedance mismatch that is hard to solve in the current version.

@vincent-fuchs
Copy link
Author

I've pushed the code in https://github.com/vincent-fuchs/cucumber-jvm-2584

Cucumber version is defined here : https://github.com/vincent-fuchs/cucumber-jvm-2584/blob/main/build.gradle#L65 , you can see that when upgrading to 7.4.1 , the error message is different.

I am actuallw struggling to fix the issue at startup with 7.3.4 :

Cannot invoke "io.cucumber.java8.LambdaGlueRegistry.addDataTableType(io.cucumber.core.backend.DataTableTypeDefinition)" because the return value of "java.lang.ThreadLocal.get()" is null

I wrote an equivalent test with cucumber-java instead of cucumber-java8, and I didn't have the problem.. so maybe it comes ffrom cucumber-java8 ?

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Jul 7, 2022

SpringBootTest(classes = DwhE2EApplication.class, webEnvironment = RANDOM_PORT)
@CucumberContextConfiguration
@ActiveProfiles("test")
public class DwhStepDef implements En {

Thanks for the reproducer. That made things easy.

You've annotated a lambda step definitions with a context configuration. Cucumber uses this class to start the spring application.

Cucumber also has to create an instance of this class to provide to springs test context manager framework. This instance must be created before the Java8Backend starts. So steps can't be registered yet.

This is partially due to the impedance mismatch but also yet another reason for #2279. Registering steps at runtime causes all sorts of problemens.

As a workaround consider putting the context configuration on it's own class.

@mpkorstanje
Copy link
Contributor

As a fix I'd consider

  1. A better exception when an attempt is made to register steps but not scenario is running (i.e. fix the NPE).
  2. And some internal state for SpringTestContextAdaptor so we don't try to get a test instance when none was created.

@vincent-fuchs
Copy link
Author

Thanks a lot for the pointer to the workaround !

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

Successfully merging a pull request may close this issue.

2 participants