From 6672a02451bbac50b2cbd72b18b75f8f98b9f70e Mon Sep 17 00:00:00 2001
From: Marc Dumais <marc.dumais@ericsson.com>
Date: Wed, 1 Feb 2023 13:29:38 -0500
Subject: [PATCH] [browser tests] Misc improvements in test infrastructure

Re-use puppeteer's empty Chrome tab:

Puppeteer at some point started to open the Chrome browser with an empty
tab by default. Then, when we create a new page for our tests, a new tab
would be added. At least When running the tests with UI
(option --test-inspect), sometimes the empty tab would be selected and
apparently cause many tests to fail. It's possible that this also sometimes
happened in headless mode.

To avoid this, now re-use the empty tab instead of creating a new one.

Make sure the Theia test app has focus and clear local browser storage:

When launching in non-headless mode (with a UI and dev-tools open), it looks-like
the dev-tools have focus, which interferes with some tests that query the UI,
expecting our app to be in focus. Simply clicking on the app before starting the
tests fixes it.

Also clear the local browser storage, to possibly avoid starting the app
with some state from the previous run. This could help when running the tests
locally, since CI should in theory always start with a clean environment.

Disable retry for failed tests in mocha config:

It only seems to make things worse.

Allow vieport to take available space instead of default 800x600:

This gives much more space for the Theia app, specially when running
in non-headless mode, where the editors can have very little real-estate
when views on the sides are open.

Delay exit after test finish:

When running the suite in headless mode, it exits as soon as the mocha tests
are done executing. In some cases, where there are lots of errors to report[1],
the final report did not have the chance to be printed-out, and so we were
left wondering about the details of what caused the failures.

[1]: basically the final pass/not pass/skip count as well as details about
the failed tests.

note: includes a review suggestion by paul-marechal.

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
---
 dev-packages/cli/src/run-test.ts  | 22 +++++++++++++++++-----
 dev-packages/cli/src/test-page.ts |  2 +-
 dev-packages/cli/src/theia.ts     |  2 ++
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/dev-packages/cli/src/run-test.ts b/dev-packages/cli/src/run-test.ts
index b3284dd19b9ef..76a004c08e5cc 100644
--- a/dev-packages/cli/src/run-test.ts
+++ b/dev-packages/cli/src/run-test.ts
@@ -36,15 +36,25 @@ export default async function runTest(options: TestOptions): Promise<void> {
         matchAppUrl: () => true, // all urls are application urls
         newPage: async () => {
             const browser = await puppeteer.launch(launch);
-            return browser.newPage();
+            // re-use empty tab
+            const [tab] = await browser.pages();
+            return tab;
         },
         onWillRun: async () => {
+            const promises = [];
             if (options.coverage) {
-                await Promise.all([
-                    testPage.coverage.startJSCoverage(),
-                    testPage.coverage.startCSSCoverage()
-                ]);
+                promises.push(testPage.coverage.startJSCoverage());
+                promises.push(testPage.coverage.startCSSCoverage());
+            }
+            // When launching in non-headless mode (with a UI and dev-tools open), make sure
+            // the app has focus, to avoid failures of tests that query the UI's state.
+            if (launch && launch.devtools) {
+                promises.push(testPage.waitForSelector('#theia-app-shell.p-Widget.theia-ApplicationShell')
+                    .then(e => e.click()));
             }
+            // Clear application's local storage to avoid reusing previous state
+            promises.push(testPage.evaluate(() => localStorage.clear()));
+            await Promise.all(promises);
         },
         onDidRun: async failures => {
             if (options.coverage) {
@@ -56,6 +66,8 @@ export default async function runTest(options: TestOptions): Promise<void> {
                 require('puppeteer-to-istanbul').write([...jsCoverage, ...cssCoverage]);
             }
             if (exit) {
+                // allow a bit of time to finish printing-out test results
+                await new Promise(resolve => setTimeout(resolve, 1000));
                 await testPage.close();
                 process.exit(failures > 0 ? 1 : 0);
             }
diff --git a/dev-packages/cli/src/test-page.ts b/dev-packages/cli/src/test-page.ts
index 29e50554f2a10..f94ddedb618da 100644
--- a/dev-packages/cli/src/test-page.ts
+++ b/dev-packages/cli/src/test-page.ts
@@ -113,7 +113,7 @@ export default async function newTestPage(options: TestPageOptions): Promise<pup
                 reporter: 'spec',
                 ui: 'bdd',
                 color: true,
-                retries: 5
+                retries: 0
             });
         });
 
diff --git a/dev-packages/cli/src/theia.ts b/dev-packages/cli/src/theia.ts
index 3a5df9dc57848..0a7e38b80cf55 100644
--- a/dev-packages/cli/src/theia.ts
+++ b/dev-packages/cli/src/theia.ts
@@ -525,6 +525,8 @@ async function theiaCli(): Promise<void> {
                     }),
                     launch: {
                         args: ['--no-sandbox'],
+                        // eslint-disable-next-line no-null/no-null
+                        defaultViewport: null, // view port can take available space instead of 800x600 default
                         devtools: testInspect
                     },
                     files: {