E2E test fixing and bring back reports (#9238)

After investigating some errors, I found another two missing awaits in our tests. Because those are so easy to overlook, I added a lint rule which makes failure on unhandled promise (for e2e tests only).

Also, enabled HTML reports again, with traces this time, to enable closer investigation of any failure in the future. @mwu-tow added code for uploading them in GH.

app/gui2/e2e/collapsingAndEntering.spec.ts

@@ -89,7 +89,7 @@ test('Collapsing nodes', async ({ page }) => {
   await customExpect.toExist(locate.graphNodeByBinding(page, 'sum'))
   await customExpect.toExist(locate.graphNodeByBinding(page, 'prod'))
 
-  locate
+  await locate
     .graphNodeByBinding(page, 'ten')
     .locator('.icon')
     .click({ modifiers: ['Shift'] })

app/gui2/e2e/componentBrowser.spec.ts

@@ -133,7 +133,7 @@ test('Filling input with suggestions', async ({ page }) => {
   )
 
   // Applying suggestion
-  page.keyboard.press('Tab')
+  await page.keyboard.press('Tab')
   await customExpect.toExist(locate.componentBrowser(page))
   await expect(locate.componentBrowserInput(page).locator('input')).toHaveValue(
     'Standard.Base.Data.read ',

app/gui2/e2e/fullscreenVisualisation.spec.ts

@@ -4,20 +4,12 @@ import * as customExpect from './customExpect'
 import * as locate from './locate'
 import { graphNodeByBinding } from './locate'
 
-/**
- * Prepare the graph for the tests.
- */
-async function initGraph(page: Page) {
-  await actions.goToGraph(page)
-}
-
 /**
  Scenario: We open the default visualisation of the `aggregated` node. We then make it fullscreen and expect it to show
  the JSON data of the node. We also expect it to cover the whole screen and to have a button to exit fullscreen mode.
  */
 test('Load Fullscreen Visualisation', async ({ page }) => {
-  await initGraph(page)
-
+  await actions.goToGraph(page)
   const aggregatedNode = graphNodeByBinding(page, 'aggregated')
   await aggregatedNode.click()
   await page.keyboard.press('Space')

app/gui2/eslint.config.js

@@ -9,7 +9,14 @@ const DIR_NAME = path.dirname(url.fileURLToPath(import.meta.url))
 
 const conf = [
   {
-    ignores: ['rust-ffi/pkg', 'rust-ffi/node-pkg', 'dist', 'shared/ast/generated', 'templates'],
+    ignores: [
+      'rust-ffi/pkg',
+      'rust-ffi/node-pkg',
+      'dist',
+      'shared/ast/generated',
+      'templates',
+      'playwright-report',
+    ],
   },
   ...compat.extends('plugin:vue/vue3-recommended'),
   eslintJs.configs.recommended,
@@ -50,6 +57,16 @@ const conf = [
       'vue/multi-word-component-names': 0,
     },
   },
+  // We must make sure our E2E tests await all steps, otherwise they're flaky.
+  {
+    files: ['e2e/**/*.spec.ts'],
+    languageOptions: {
+      parser: await import('@typescript-eslint/parser'),
+    },
+    rules: {
+      '@typescript-eslint/no-floating-promises': 2,
+    },
+  },
 ]
 
 export default conf

app/gui2/playwright.config.ts

@@ -39,13 +39,15 @@ export default defineConfig({
   forbidOnly: !!process.env.CI,
   repeatEach: process.env.CI ? 3 : 1,
   ...(process.env.CI ? { workers: 1 } : {}),
+  timeout: 60000,
   expect: {
     timeout: 5000,
     toHaveScreenshot: { threshold: 0 },
   },
+  reporter: 'html',
   use: {
     headless: !DEBUG,
-    trace: 'on-first-retry',
+    trace: 'retain-on-failure',
     viewport: { width: 1920, height: 1600 },
     ...(DEBUG
       ? {}

app/ide-desktop/eslint.config.js

@@ -244,7 +244,7 @@ export default [
     eslintJs.configs.recommended,
     {
         // Playwright build cache.
-        ignores: ['**/.cache/**'],
+        ignores: ['**/.cache/**', '**/playwright-report'],
     },
     {
         settings: {

app/ide-desktop/lib/dashboard/playwright.config.ts

@@ -19,8 +19,10 @@ export default test.defineConfig({
     timeout: 30_000,
   },
   timeout: 30_000,
+  reporter: 'html',
   use: {
     baseURL: 'http://localhost:8080',
+    trace: 'retain-on-failure',
     launchOptions: {
       ignoreDefaultArgs: ['--headless'],
       args: [

build/build/paths.yaml

@@ -17,6 +17,7 @@
     gui/:
     gui2/: # The new, Vue-based GUI.
       dist/:
+      playwright-report/: # Test results for the new GUI that we want to keep as artifacts.
       public/:
         font-enso/:
         font-mplus1/:
@@ -33,6 +34,8 @@
         content-config/:
           src/:
             config.json:
+        dashboard/:
+          playwright-report/: # Test results for the dashboard that we want to keep as artifacts.
         icons/:
         project-manager/:
   build/:

build/build/src/engine/context.rs

@@ -233,7 +233,7 @@ impl RunContext {
         ];
         for (argfile, artifact_name) in native_image_arg_files {
             if argfile.exists() {
-                ide_ci::actions::artifacts::upload_single_file(&argfile, artifact_name).await?;
+                ide_ci::actions::artifacts::upload_single_file(argfile, artifact_name).await?;
             } else {
                 warn!(
                     "Native Image Arg File for {} not found at {}",

build/ci_utils/src/actions/artifacts.rs

@@ -12,6 +12,7 @@ use serde::de::DeserializeOwned;
 use tempfile::tempdir;
 
 
+
 // ==============
 // === Export ===
 // ==============
@@ -67,36 +68,47 @@ pub fn discover_recursive(
     rx.into_stream()
 }
 
-pub async fn upload(
+
+pub fn upload(
     file_provider: impl Stream<Item = FileToUpload> + Send + 'static,
-    artifact_name: impl AsRef<str>,
+    artifact_name: impl Into<String>,
     options: UploadOptions,
-) -> Result {
-    let handler =
-        ArtifactUploader::new(SessionClient::new_from_env()?, artifact_name.as_ref()).await?;
-    let result = handler.upload_artifact_to_file_container(file_provider, &options).await;
-    // We want to patch size even if there were some failures.
-    handler.patch_artifact_size().await?;
-    result
+) -> BoxFuture<'static, Result> {
+    let artifact_name = artifact_name.into();
+    let span = info_span!("Artifact upload", artifact_name);
+    async move {
+        let handler = ArtifactUploader::new(SessionClient::new_from_env()?, artifact_name).await?;
+        let result = handler.upload_artifact_to_file_container(file_provider, &options).await;
+        // We want to patch size even if there were some failures.
+        handler.patch_artifact_size().await?;
+        result
+    }
+    .instrument(span)
+    .boxed()
 }
 
 pub fn upload_single_file(
     file: impl Into<PathBuf>,
-    artifact_name: impl AsRef<str>,
-) -> impl Future<Output = Result> {
+    artifact_name: impl Into<String>,
+) -> BoxFuture<'static, Result> {
     let file = file.into();
-    let files = single_file_provider(file);
-    (async move || -> Result { upload(files?, artifact_name, default()).await })()
+    let artifact_name = artifact_name.into();
+    info!("Uploading file {} as artifact {artifact_name}.", file.display());
+    single_file_provider(file)
+        .and_then_async(move |stream| upload(stream, artifact_name, default()))
+        .boxed()
 }
 
 pub fn upload_directory(
     dir: impl Into<PathBuf>,
-    artifact_name: impl AsRef<str>,
-) -> impl Future<Output = Result> {
+    artifact_name: impl Into<String>,
+) -> BoxFuture<'static, Result> {
     let dir = dir.into();
-    info!("Uploading directory {}.", dir.display());
-    let files = single_dir_provider(&dir);
-    (async move || -> Result { upload(files?, artifact_name, default()).await })()
+    let artifact_name = artifact_name.into();
+    info!("Uploading directory {} as artifact {artifact_name}.", dir.display());
+    single_dir_provider(&dir)
+        .and_then_async(move |stream| upload(stream, artifact_name, default()))
+        .boxed()
 }
 
 #[tracing::instrument(skip_all , fields(artifact_name = %artifact_name.as_ref(), target = %target.as_ref().display()), err)]

build/cli/src/lib.rs

@@ -23,7 +23,6 @@
 pub mod arg;
 
 
-
 pub mod prelude {
     pub use crate::arg::ArgExt as _;
     pub use enso_build::prelude::*;
@@ -105,6 +104,27 @@ fn resolve_artifact_name(input: Option<String>, project: &impl IsTarget) -> Stri
     input.unwrap_or_else(|| project.artifact_name())
 }
 
+/// Run the given future. After it is complete (regardless of success or failure), upload the
+/// directory as a CI artifact.
+///
+/// Does not attempt any uploads if not running in a CI environment.
+pub fn run_and_upload_dir(
+    fut: BoxFuture<'static, Result>,
+    dir_path: impl Into<PathBuf>,
+    artifact_name: impl Into<String>,
+) -> BoxFuture<'static, Result> {
+    let dir_path = dir_path.into();
+    let artifact_name = artifact_name.into();
+    async move {
+        let result = fut.await;
+        if is_in_env() {
+            ide_ci::actions::artifacts::upload_directory(dir_path, artifact_name).await?;
+        }
+        result
+    }
+    .boxed()
+}
+
 define_env_var! {
     ENSO_BUILD_KIND, enso_build::version::Kind;
 }
@@ -350,10 +370,20 @@ impl Processor {
         match gui.command {
             arg::gui2::Command::Build(job) => self.build(job),
             arg::gui2::Command::Get(source) => self.get(source).void_ok().boxed(),
-            arg::gui2::Command::Test =>
-                try_join(gui2::tests(&self.repo_root), gui2::dashboard_tests(&self.repo_root))
-                    .void_ok()
-                    .boxed(),
+            arg::gui2::Command::Test => {
+                let repo_root = self.repo_root.clone();
+                let gui_tests = run_and_upload_dir(
+                    gui2::tests(&repo_root),
+                    &repo_root.app.gui_2.playwright_report,
+                    "gui2-playwright-report",
+                );
+                let dashboard_tests = run_and_upload_dir(
+                    gui2::dashboard_tests(&repo_root),
+                    &repo_root.app.ide_desktop.lib.dashboard.playwright_report,
+                    "dashboard-playwright-report",
+                );
+                try_join(gui_tests, dashboard_tests).void_ok().boxed()
+            }
             arg::gui2::Command::Watch => gui2::watch(&self.repo_root),
             arg::gui2::Command::Lint => gui2::lint(&self.repo_root),
         }