Testing too large file dimension #3182

Cerchie · 2025-12-30T19:20:47Z

Summary of Changes

This draft adds a third dimension (files that should fail) to the artifact E2E tests.

The first thing was adding createLargeFile and cleanupLargeFile to the test utils. I chose to take the approach of creating and cleaning up the file rather than keeping it in a fixture, so the functionality belonged in utils, not fixtures.

Next, I originally implemented the matrix "entrypoint × provider/region × file size" with a triple for loop, but that felt excessive, so I mapped the entrypoints and provider regions to success/failure cases for the file size.

If you look at the invalid file dimension PR you can see that the array for file types (valid, abnormal size) is now well set up for adding the last dimension, invalid JAR files.

Note

This PR is was branched off cerchie/renaming which is now in main

%%{init: {'theme': 'base'}}%%
  gitGraph
      commit id: "103f07e9 (main)"

      branch cerchie/renaming
      checkout cerchie/renaming
      commit id: "7ae2b93346678978c9480206223f295b2b005c3b"

      branch cerchie/large-file
      checkout cerchie/large-file
      commit id: "a4740a84"

      branch cerchie/invalide-file-dimension
      checkout cerchie/invalide-file-dimension
      commit id: "ef22ed07"

Pull request checklist

Please check if your PR fulfills the following (if applicable):

Tests

Added new
Updated existing
Deleted existing

Release notes

Does anything in this PR need to be mentioned in the user-facing CHANGELOG?

Cerchie · 2025-12-31T15:54:58Z

tests/e2e/specs/flinkArtifact.spec.ts

  artifactPath: string,
  artifactsView: FlinkDatabaseView,
 ): Promise<string> {
-  const uploadedArtifactName = await artifactsView.uploadFlinkArtifact(electronApp, artifactPath);


felt redundant

Copilot

Pull request overview

This PR adds test coverage for large file upload rejection in Flink artifact uploads. The changes introduce a new utility for creating large test files and extend the existing E2E test suite to verify that artifacts exceeding 100MB are properly rejected.

Key changes:

Added utility functions to create and cleanup large test files for testing upload size limits
Restructured tests to support a test matrix covering different entrypoints, cloud providers, and file sizes
Refactored helper functions to accept dynamic file paths instead of using hardcoded fixture paths

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
tests/e2e/utils/flinkDatabase.ts	New utility module providing functions to create and cleanup large test files for artifact upload testing
tests/e2e/specs/flinkArtifact.spec.ts	Extended test suite with file size validation tests, restructured into test matrix pattern, and updated helper functions to accept dynamic file paths
tests/e2e/objects/views/FlinkDatabaseView.ts	Renamed methods for clarity and improved documentation to better describe their purpose

tests/e2e/utils/flinkDatabase.ts

tests/e2e/specs/flinkArtifact.spec.ts

Cerchie · 2025-12-31T16:19:54Z

tests/e2e/specs/flinkArtifact.spec.ts

+  const fileSizes = [
+    {
+      description: "valid size artifact",
+      setupFile: () => artifactPath,


just returns a string, decided to go with an inline func for consistency with the next object in the fileSizes array

Cerchie · 2025-12-31T19:35:39Z

tests/e2e/specs/flinkArtifact.spec.ts

+            // Swallow any errors from the upload flow since we expect failure
+          }
+
+          await expect(artifactsView.artifacts).toHaveCount(initialArtifactCount);


we assert based on the artifact count, because the name will never have been created at this point, so it will always not be found

Cerchie · 2025-12-31T19:36:26Z

tests/e2e/utils/flinkDatabase.ts

@@ -0,0 +1,54 @@
+import * as fs from "fs";


new file, will also set the stage for adding a util that create an invalid JAR file for the next dimension

Cerchie · 2026-01-01T16:29:46Z

tests/e2e/specs/flinkArtifact.spec.ts

+    //     /* no cleanup needed for fixture file */
+    //   },
+    //   shouldSucceed: true,
+    // },


only one for now, but see the next PR

Cerchie · 2026-01-02T18:37:15Z

tests/e2e/objects/views/FlinkDatabaseView.ts

   * @param electronApp - The Electron application instance
   * @param filePath - The path to the JAR file to upload
   * @param skipInitiation - If true, skips clicking the upload button (assumes quickpick is already open)
+   * @param expectSuccess - If true, waits for success notification; if false, caller handles error notification


added since we now expect failures sometimes

Cerchie · 2026-01-02T22:21:45Z

tests/e2e/specs/flinkArtifact.spec.ts

    {
      entrypoint: SelectFlinkDatabase.DatabaseFromResourcesView,
-      testName: "should upload Flink Artifact when cluster selected from the Resources view",
+      testName: "cluster selected from the Resources view",


not successful every time now

shouples

Good progress overall! I'm mainly concerned about the expectSuccess usage requiring some mental gymnastics to follow the test flow/logic and think it could be simplified a bit, and then (to a lesser extent) any lingering big .jar files that are created but not explicitly cleaned up. We can address those in a near-term follow-up though.

shouples · 2026-01-07T14:58:19Z

tests/e2e/objects/views/FlinkDatabaseView.ts

+    if (expectSuccess) {
+      // Wait for upload success notification
+      const notificationArea = new NotificationArea(this.page);
+      const successNotifications = notificationArea.infoNotifications.filter({
+        hasText: "uploaded successfully",
+      });
+      await expect(successNotifications.first()).toBeVisible();
+    }


Doesn't have to be in this branch, but I think we should move these out into the individual tests themselves so too much test-logic isn't inside of these page object models.

shouples · 2026-01-07T15:37:44Z

tests/e2e/specs/flinkArtifact.spec.ts

            provider,
            region,
+            pathToBigArtifact,
+            false, // expectSuccess - we expect this upload to fail


This feels a little odd - what errors do we expect here? It may be clearer to expect the error notification instead of handling the failed info-notification assertion

tests/e2e/specs/flinkArtifact.spec.ts

shouples · 2026-01-07T15:41:04Z

tests/e2e/utils/flinkDatabase.ts

+ */
+export function createLargeFile(options: LargeFileOptions = {}): string {
+  const {
+    sizeInMB = 101,


Minor: it may be clearer to set the size limit the extension uses as a top-level constant here, then just increment by 1 to indicate exceeding the size

Suggested change

sizeInMB = 101,

sizeInMB = MAX_ARTIFACT_FILE_SIZE_MB + 1, // just over the limit to trigger error notification

Cerchie added 6 commits December 30, 2025 11:23

update names

04ba678

remove unrelated file

7ae2b93

first stab

9e39402

few flaky tests left

a95e196

update filepath for fixture

35c4e3c

move fixtures file to utils

000f695

Cerchie commented Dec 31, 2025

View reviewed changes

Cerchie requested a review from Copilot December 31, 2025 15:55

Copilot AI reviewed Dec 31, 2025

View reviewed changes

tests/e2e/utils/flinkDatabase.ts Outdated Show resolved Hide resolved

tests/e2e/utils/flinkDatabase.ts Outdated Show resolved Hide resolved

tests/e2e/specs/flinkArtifact.spec.ts Outdated Show resolved Hide resolved

tests/e2e/specs/flinkArtifact.spec.ts Show resolved Hide resolved

Cerchie added 2 commits December 31, 2025 09:07

copilot

ab1629b

remove async

a4740a8

Cerchie commented Dec 31, 2025

View reviewed changes

Cerchie added 2 commits December 31, 2025 10:48

remove triple nested for loop and make more readable

4132cac

update desc

29a2eff

Cerchie changed the base branch from main to cerchie/renaming December 31, 2025 18:21

Cerchie changed the title ~~DRAFT: Testing large file dimension~~ Draft: Testing large file dimension Dec 31, 2025

Cerchie changed the title ~~Draft: Testing large file dimension~~ Draft: Testing too large file dimension Dec 31, 2025

Cerchie commented Dec 31, 2025

View reviewed changes

Cerchie added 2 commits December 31, 2025 13:08

add boolean for tracking failur

db00d4b

make comment accurate

bfe87f8

Cerchie changed the title ~~Draft: Testing too large file dimension~~ Testing too large file dimension Dec 31, 2025

Cerchie added 3 commits January 1, 2026 07:36

rename file configs

297948b

remove success file

2d26ddb

remove comment

299e062

Cerchie commented Jan 1, 2026

View reviewed changes

Base automatically changed from cerchie/renaming to main January 2, 2026 15:46

merge from main

30f967a

Cerchie commented Jan 2, 2026

View reviewed changes

Cerchie marked this pull request as ready for review January 2, 2026 19:09

Cerchie requested a review from a team as a code owner January 2, 2026 19:09

Cerchie added 2 commits January 2, 2026 13:45

working toward a refactor

fb8bf64

update with cleanup

1f6942a

Cerchie commented Jan 2, 2026

View reviewed changes

shouples self-assigned this Jan 5, 2026

shouples approved these changes Jan 7, 2026

View reviewed changes

Cerchie merged commit ea83fc1 into main Jan 7, 2026
14 checks passed

Cerchie deleted the cerchie/large-file branch January 7, 2026 17:17

	sizeInMB = 101,
	sizeInMB = MAX_ARTIFACT_FILE_SIZE_MB + 1, // just over the limit to trigger error notification

Testing too large file dimension #3182

Testing too large file dimension #3182

Uh oh!

Conversation

Cerchie commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of Changes

Pull request checklist

Tests

Release notes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Cerchie Dec 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shouples left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Cerchie commented Dec 30, 2025 •

edited

Loading

Cerchie Dec 31, 2025 •

edited

Loading