vaadin / testbench

Vaadin TestBench is a tool for automated user interface testing of Vaadin applications.
https://vaadin.com/testbench
Other
20 stars 22 forks source link

Quarkus support in testbench #1655

Open ErrorProne opened 1 year ago

ErrorProne commented 1 year ago

What do I want

Working quarkus support for pro features like UiUnitTest

Explain your problem

Two things, based on who reads this

Testbench-Devs: There are multiple problems we found with quarkus support so far:

  1. The route-lookup does not work
  2. Quarkus @TestSecurity (https://quarkus.io/guides/security-testing) feature is not working (similiar to springs @WithMockUser)
  3. I'm not 100% sure about that, but it looks like the BeanInstantiator also has to be wired by hand.

All of those should be solved with the workarounds in [1] but an official out-of-the-box support should be the goal.

The vaadin project and team does such an amazing work and the framework is wonderful to use. The same should hold true for testing.

We will be happy to provide feedback or input to any implementations for this.

Project owner/Pro feature Support: A little rant: Before switching to Vaadin we talked to a few folks and very explicitly stated that we do test a lot and use quarkus. We do deploy applications into factory lines. These apps have to work stable, no exceptions. Naturally we were very interested in the pro Testbench features and have been told that quarkus is supported. Our experience right now is: that is not the case at all. We got it working eventually but only by diving into the vaadin code and with help from Marco Collovati in this issue in the quarkus extension here [1].

So far it looks like we solved all the issues in [1] but for a pro feature with official quarkus support and no mention about these problems at all (e.g. https://vaadin.com/docs/latest/integrations/quarkus) is a very frustrating experience. Additionally at least I see testing in 2023 not as an nice-to-have addition.

[1] https://github.com/vaadin/quarkus/issues/121

tarekoraby commented 1 year ago

Thank you @ErrorProne for the ticket. The TestBench team, I'm sure, will address the technical feasibility of the proposed solution.

But as the product manager, my sincere apologies for any false promises made regarding TestBench support with Quarkus. It is indeed reasonable to expect TestBench to work with Quarkus without the need to resort to any workarounds. And if, for some reason, this support turns out to be impossible or prohibitively expensive to implement, we should clearly emphasize that in the Quarkus and TestBench documentation pages.

In all cases, we will aim to investigate possible ways of improving TestBench + Quarkus support.

benjaminrau commented 1 year ago

Thanks for your reply @tarekoraby - it would be really awesome to see some of our workarounds to make their way into the official Vaadin Testbench implementation.

I forked the afaik official starter project for Quarkus + Vaadin and extended it with workarounds for all of the issues we identified and fixed so far.

You find it here: https://github.com/AliceAndTheBuilders/pro-starter-flow-quarkus

If some comments reads as offense - lets make me clear that this is not my intention and just read them with positive vibes.

We adressed the following issues so far:

  1. auto discovering and registering vaadin routes
  2. UI Unit Tests for quarkus with authentication support
  3. Using UIUnitTest while having another base test class one may want to extend (which is not UI related)
  4. register and initialize ViewAccessChecker
  5. Login form for Quarkus
  6. @PermitAll in Vaadin is a synonym for @RolesAllowed("**") which does not match Quarkus definition
tarekoraby commented 1 year ago

Thanks for sharing the workarounds 🙏 That's very helpful.

It's the summer holiday season here in Finland, but we will get back to you as soon as we can once the bulk of our developers return and had the opportunity to examine the issue.

ErrorProne commented 1 year ago

TestBench End-To-End is also not working. Some of the problems seem to be related to how quarkus handles isolation of extensions with different classloaders. It looks like you guys should talk to your quarkus folks about that.

I think that we figured out a workaround. We will update the starter later with the new workaround and keep you posted.

fjasis commented 1 year ago

In the starter @benjaminrau mentioned, why is needed for the injected properties to be use in @PostConstruct? it is possible to use them outside of the postconstruct?

ErrorProne commented 1 year ago

In the starter @benjaminrau mentioned, why is needed for the injected properties to be use in @PostConstruct? it is possible to use them outside of the postconstruct?

I'm not sure if I understand the question. But I guess you are referring to this? https://github.com/AliceAndTheBuilders/pro-starter-flow-quarkus/blob/v24/src/main/java/com/example/starter/pro/ProtectedView.java#L20

He is using the @Inject on a field level. This means the field is not injected when the constructor runs. That is why he switched to @PostConstruct. You could use constructor injection and than use a normal constructor and access the injects.

fjasis commented 1 year ago

Yes, that was my question, sorry i worded it poorly, so this should work rigth? SecurityIdentityAssociation identity public testConstructor(SecurityIdentityAssociation identity){ this.identity = identity; }

I'm having trouble formatting it properly, but i think it gets the point across

ErrorProne commented 1 year ago

Yes, as per https://quarkus.io/guides/cdi-reference#simplified-constructor-injection this should work (given that all other things are right). For the starter example this would be

    transient SecurityIdentityAssociation identity;

    public ProtectedView(SecurityIdentityAssociation identity) {
        this.identity = identity;
        textField.setValue(identity.getIdentity().getPrincipal().getName());
    }
fjasis commented 1 year ago

Thanks! i tested it yesterday and it works correctly with JPA-security, what is the reason the field injection needs to be used at post construct with vaadin? also, as how are you handling logouts?

ErrorProne commented 1 year ago

Thanks! i tested it yesterday and it works correctly with JPA-security, what is the reason the field injection needs to be used at post construct with vaadin? also, as how are you handling logouts?

This is not really a vaadin specific thing. Field injection works in a way where the container (I guess Quarkus ArC in this case) first instantiates your class through the no args constructor and than sets the injects fields via setters. This basically means that the fields will be injected after the constructor has finished. So when field injection is used you have to opt-in for the @PostContruct event. When constructor injection is used instead you have instant access in the injected values inside the constructor, but you have to set the fields by yourself.

Both are valid methods for injection and have a long history of discussions of pros/cons. As far as I'm aware constructor injection is more favoured by tools like sonarqube etc. since they make the classes for example "easier" testable. But you will find people who vote for each variant.

Do you have anything specific in mind regarding logouts? Also feel free to create an issue on our starter fork (https://github.com/AliceAndTheBuilders/pro-starter-flow-quarkus) so we can keep this issue on topic =)

fjasis commented 1 year ago

Issue Created!

benjaminrau commented 1 year ago

@fjasis I created a pull request which adds a logout example: https://github.com/AliceAndTheBuilders/pro-starter-flow-quarkus/pull/2

And from what i see, is Vaadin on Quarkus not managing any sessions itself, which would require cleanup. In Quarkus when using form auth all session information is on the encrypted cookie as Finn mentioned, thus as soon as it is deleted, all session information is gone.

Assuming that, the example should be sufficient.

ErrorProne commented 1 year ago

For End-To-End tests a workaround is to change the lookup method for required modules in the tests application.properties (important, it requires .properties even if yaml is used!). For example:

quarkus.class-loading.parent-first-artifacts=com.vaadin:vaadin-testbench-core-junit5,com.vaadin:vaadin-testbench-shared,org.seleniumhq.selenium:selenium-api

For vaadin devs workin on this, please see https://quarkus.io/guides/class-loading-reference#isolated-classloaders

I'lltry to push an example to our forked starter later this week

benjaminrau commented 1 year ago

My observations on the issue which is worked around as @ErrorProne mentioned are the following per example:

  1. @BrowserTest invokes the JUnit MultipleBrowsersExtension which creates an instance of BrowerTestInfo to resolve the parameter on BrowserTestBase::setBrowserTestInfo
  2. This is an instance of BrowserTestBase which is registered on the isolated class loader
  3. @QuarkusTest invokes the QuarkusTestExtension which somewhat intercepts any paramter resolvers for fx @BeforeEach
  4. On the method io.quarkus.test.junit.QuarkusTestExtension#runExtensionMethod() there are some checks if an parameter needs to be cloned for a reason that is described like: the arguments were not loaded from TCCL so we need to deep clone them into the TCCL because the test method runs from a class loaded from the TCCL
  5. Indeed for the BrowserTestInfo instance the following check results in cloneRequired is true: cloneRequired = runningQuarkusApplication.getClassLoader().loadClass(theclass.getName()) != theclass;
  6. Later the BrowserTestInfo instance is tried to be cloned with xStream which fails since xStream is performing a lot of illegal access on java internals

By instructing Quarkus to NOT use the isolated class loader the check for cloneRequired (5) is false and QuarkusTestExtension will not try to clone the instance.

The following comment on the docs at https://quarkus.io/guides/writing-extensions#using-threads-context-class-loader seems related:

The build step will be run with a TCCL that can load user classes from the deployment in a transformer-safe way. This class loader only lasts for the life of the augmentation, and is discarded afterwards. The classes will be loaded again in a different class loader at runtime. This means that loading a class during augmentation does not stop it from being transformed when running in the development/test mode.

Sidenote related to the autodiscover routes feature on Testbench Unit Tests: Since beeing aware of the class loader topic i think that this class loader issue is also accountable for the issue that no routes are registered on unit tests as we pointed out earlier on this issue.

mshabarov commented 1 year ago

Thanks @benjaminrau , @ErrorProne for providing your codes and all who commented on this issue. I just want to share our plans about further work - Vaadin Flow team keeps an eye on this topic and we are going to start working on it step-by-step having UI Unit Testing better integration first and then checking the end-to-end testing. This would require an analysis of proposed workarounds and decomposing to a smaller pieces of work that we can do iteratively. I expect this work to start on upcoming fall 2023, closer to October.

mshabarov commented 10 months ago

It took a while for me to triage this ticket and split into smaller parts, here what I eventually get (in priority order):

Two other issues regarding AtPermitAll annotation and UIUnitTest base class are more general developer experience issue and can be considered later after we fix the above bug.

Vaadin Flow team will work on fixing these issue during normal maintenance one by one, I believe they can be done separately. I'll keep you posted about results.

Thanks again for the workarounds, much helpful!

mcollovati commented 8 months ago

One issue with running UI Unit Tests with @QuarkusTest is that the HTTP server is started even if it should not be needed.

Quarkus 3.2 introduced the @QuarkusComponentTest annotation for testing with CDI (https://quarkus.io/blog/quarkus-component-test/). Unfortunately, this doesn't seem to play well with UI Unit testing, since a lot of manual setup is required.

@ErrorProne in the provided example repository, the UI Unit tests are also starting the HTTP server. Did you manage in some way to prevent running the whole application and just focus on specific components?