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

Each TestBench command for a Fusion app takes 20s with Flow 6 alpha 5 #9905

Closed
Artur- opened this issue Jan 28, 2021 · 11 comments · Fixed by #10014 or #11678
Closed

Each TestBench command for a Fusion app takes 20s with Flow 6 alpha 5 #9905

Artur- opened this issue Jan 28, 2021 · 11 comments · Fixed by #10014 or #11678

Comments

@Artur-
Copy link
Member

Artur- commented Jan 28, 2021

Description of the bug / feature

The waiting logic in TestBench goes partly like

if (window.Vaadin.Flow && !window.Vaadin.Flow.clients) {
  // Wait for the ApplicationConnection to show up
}

This no longer works at all in Fusion apps with Flow 6 alpha 5 because the application theme is set as

window.Vaadin.Flow._vaadintheme_something

apparently also the DevModeGizmo is set in dev mode as

window.Vaadin.Flow.devModeGizmo

The result is that each and every command in a test takes 20s, which is the timeout for waiting

Not sure if this should be fixed in Flow or TestBench

@Artur- Artur- changed the title Each TestBench command takes 20s with Flow 6 alpha 5 Each TestBench command for a Fusion app takes 20s with Flow 6 alpha 5 Jan 28, 2021
@Legioth
Copy link
Member

Legioth commented Jan 28, 2021

Both those features are active even if not using any Flow views, so it would be logical that they would be in the window.Vaadin namespace rather than window.Vaadin.Flow.

@pleku
Copy link
Contributor

pleku commented Jan 28, 2021

Would be nice to have a smoketest in Flow that checks that the wait the TB test execution start doesn't take too long for any Vaadin app.

@taefi
Copy link
Contributor

taefi commented Feb 4, 2021

The wait for Vaadin script in TestBench is like this:

if (!window.Vaadin || !window.Vaadin.Flow) {

And after moving _vaadintheme_something and devModeGizmo to Vaadin namespace, Flow object is only initialized for clients object.

@pleku pleku moved this from P1 - High priority to WIP in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Feb 4, 2021
@taefi
Copy link
Contributor

taefi commented Feb 5, 2021

The problem was that, in the TS only version, the rendered elements are being created in the shadow root and then statements like $(TextFieldElement.class) or $(ButtonElement.class) will fail since TB fails to find the associated elements. If these statements are used within a condition passed as a lambda to waitUntil(...), they will not fail with NoSuchElementException, but will wait until a timeout happens.

By downloading a simple application from start.vaadin.com containing the default Hello World view, it can be validated that the following test would pass without any timeout:

@Test
public void testHelloWorldView_enterNameAndClickSayHello_notificationShouldBePresentedWithProperValue() {

    getDriver().get("http://localhost:8080/hello");

    waitUntil(driver -> $("hello-world-view").first() != null);

    TestBenchElement helloWorldView = $("hello-world-view").first();

    helloWorldView.$(TextFieldElement.class).id("name-field").setValue("World!");
    helloWorldView.$(ButtonElement.class).first().click();

    waitUntil(driver -> findElement(By.tagName("vaadin-notification-card")).isDisplayed());

    Assert.assertEquals("Hello World!", findElement(By.tagName("vaadin-notification-card")).getText());
}

So I'm closing this issue as I could not see a bug in regard with using TestBench with flow and tested them in both TS only and also in Java only for Flow 6.0.0.alpha5 and 6.0-SNAPSHOT.

@taefi taefi closed this as completed Feb 5, 2021
OLD Vaadin Flow ongoing work (Vaadin 10+) automation moved this from In progress to Done - pending release Feb 5, 2021
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from WIP to Closed Feb 5, 2021
@pleku
Copy link
Contributor

pleku commented Feb 8, 2021

One the points of TB is to not have to figure out what you need to know / do to be able to write the UI tests with Vaadin. So to make sure I've understood correctly, if one comments the following line the test above:

// waitUntil(driver -> $("hello-world-view").first() != null);

Then the test will fail with NoSuchElementException for the next line

TestBenchElement helloWorldView = $("hello-world-view").first();

but there is no 20 second wait happening for the test to start executing ?

@taefi
Copy link
Contributor

taefi commented Feb 8, 2021

That waitUntil(driver -> $("hello-world-view").first() != null); is just an example and I'm not sure how often it will fail if we comment this:
// waitUntil(driver -> $("hello-world-view").first() != null);
I think there is good chance that if execution of getDriver().get("http://localhost:8080/hello"); take long enough, we may encounter NoSuchElementException for the next line:
TestBenchElement helloWorldView = $("hello-world-view").first();

That example is not about how a test should be written for TS only applications using TestBench, and it is more of a quick test to show that waitUntil() and $() methods are working for TS only applications, and gives a quick tip about the necessity of using nested $() to obtain elements which are created in shadow root in TS only applications.

No such wait (20 sec. or so) happened during smoke test. Having the application up and running, it takes 2-3 sec to execute the above test (including checking the proKey license, opening the browser, etc.).

@pleku
Copy link
Contributor

pleku commented Feb 8, 2021

Right, thanks. So there is no additional disclaimer that should be needed for testing Fusion apps for now.

@Artur-
Copy link
Member Author

Artur- commented Feb 10, 2021

Was this issue fixed or just closed?

When you tested this, did you remove the Flow part from routes.ts? Otherwise it will include a Flow app and everything works as expected. I still see the same problem with Vaadin 19 beta 1.

Test project: https://github.com/Artur-/test-fusion-testbench
Run: mvn install -Pit -Pproduction

It should not say

[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 43.937 s - in foo.MyIT

rather it should say something like

[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.746 s - in foo.MyIT

@Artur- Artur- reopened this Feb 10, 2021
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from Closed to Needs triage Feb 10, 2021
@taefi
Copy link
Contributor

taefi commented Feb 10, 2021

No, as I mentioned in the above, I couldn't see any timeout problems with the TS only app I downloaded from start.vaadin.com. I was expecting that there is no need to change/remove anything from the starter app.
I will take a look on the provided test project.

@taefi
Copy link
Contributor

taefi commented Feb 10, 2021

I'm making some changes to fix the issue, but even before that, I can say I didn't find a way for running that IT using mvn install -Pit -Pproduction to take only 3.746 s - in foo.MyIT!
After starting the application and executing getDriver().get("http://localhost:8080"); it would take something like 10-15 seconds for opening the browser and waiting for the frontend compilation to finish, and then continue to the rest of the test which is trivial. I think 13.746 s - in foo.MyIT would be more realistic! ;-)

@Artur-
Copy link
Member Author

Artur- commented Feb 10, 2021

There is no waiting for frontend compilation when you are running with -Pproduction. That happens before the server is started and the test is run

taefi added a commit that referenced this issue Feb 10, 2021
TestBench wait logic is based the existence of Flow object
and in Fusion Only apps Flow object should not be initialized.
moving _vaadintheme_something and devModeGizmo to Vaadin namespace
fixes the TB wait logic in Fusion Only apps.

Fixes: #9905
@pleku pleku moved this from Needs triage to WIP in OLD Vaadin Flow bugs & maintenance (Vaadin 10+) Feb 10, 2021
OLD Vaadin Flow bugs & maintenance (Vaadin 10+) automation moved this from WIP to Closed Feb 11, 2021
pleku pushed a commit that referenced this issue Feb 11, 2021
…#10014)

TestBench wait logic is based the existence of Flow object
and in Fusion Only apps Flow object should not be initialized.
moving _vaadintheme_something and devModeGizmo to Vaadin namespace
fixes the TB wait logic in Fusion Only apps.

Fixes: #9905
vaadin-bot pushed a commit that referenced this issue Feb 11, 2021
…#10014)

TestBench wait logic is based the existence of Flow object
and in Fusion Only apps Flow object should not be initialized.
moving _vaadintheme_something and devModeGizmo to Vaadin namespace
fixes the TB wait logic in Fusion Only apps.

Fixes: #9905
pleku pushed a commit that referenced this issue Feb 11, 2021
…#10014) (#10016)

TestBench wait logic is based the existence of Flow object
and in Fusion Only apps Flow object should not be initialized.
moving _vaadintheme_something and devModeGizmo to Vaadin namespace
fixes the TB wait logic in Fusion Only apps.

Fixes: #9905

Co-authored-by: Soroosh Taefi <taefi.soroosh@gmail.com>
Artur- added a commit that referenced this issue Aug 30, 2021
mshabarov pushed a commit that referenced this issue Aug 30, 2021
vaadin-bot pushed a commit that referenced this issue Aug 30, 2021
vaadin-bot pushed a commit that referenced this issue Aug 30, 2021
vaadin-bot added a commit that referenced this issue Aug 30, 2021
…11678) (#11687)

Fixes #9905 again. It was re-introduced in #11669

Co-authored-by: Artur <artur@vaadin.com>
vaadin-bot added a commit that referenced this issue Aug 31, 2021
…11678) (#11688)

Fixes #9905 again. It was re-introduced in #11669

Co-authored-by: Artur <artur@vaadin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment