Skip to content

[E2E] Refactor file & progress report media to use new testers & operations#3626

Open
CarsonF wants to merge 5 commits intodevelopfrom
refactor/test-files
Open

[E2E] Refactor file & progress report media to use new testers & operations#3626
CarsonF wants to merge 5 commits intodevelopfrom
refactor/test-files

Conversation

@CarsonF
Copy link
Contributor

@CarsonF CarsonF commented Jan 20, 2026

No description provided.

This forward path should help ease migration,
as utility helper functions are ported to operations.
The legacy app will still be able to call them.
New implementation uses the `Identity` facade with user ID.
So tokens & sessions are not referenced, which is good because
those are completely separate implementation details.
@coderabbitai
Copy link

coderabbitai bot commented Jan 20, 2026

📝 Walkthrough

Walkthrough

This PR refactors test infrastructure and helpers from an app-centric pattern to a curried, tester-bound operation pattern. Updates include: introducing a Got HTTP client property to the Tester interface, converting test operation helpers (file, directory) into higher-order functions returning tester-bound closures, establishing a new test/operations/directory.ts module, and migrating end-to-end test cases to use the new tester.run() and tester.apply() execution model.

Changes

Cohort / File(s) Summary
Tester Infrastructure
test/setup/gql-client/gql-execute.ts, test/setup/gql-client/graphql-tester.ts, test/setup/gql-client/root-tester.ts
Updated createExecute to accept Got HTTP client instance instead of options object; added http property to Tester interface; introduced createHttpClientForApp factory; augmented getRootTester to attach identity property from login result.
Test Operations Refactoring
test/operations/file.ts, test/operations/directory.ts
Migrated file helpers (requestFileUpload, uploadFileContents, createFileVersion, getFileNode, getFileNodeChildren) to curried, tester-bound functions; created new directory.ts module with createRootDirectory and createDirectory helpers following the same pattern.
Test Utilities
test/utility/create-app.ts, test/utility/create-graphql-client.ts, test/utility/create-directory.ts, test/utility/index.ts
Added tester property to TestApp; updated createGraphqlClient to use Got HTTP client; removed and migrated createDirectory helper to test/operations/directory.ts; removed re-export from index.ts.
E2E Test Cases
test/engagement.e2e-spec.ts, test/file.e2e-spec.ts, test/progress-report-media.e2e-spec.ts
Updated test imports and calls to use new tester-bound operations; replaced app-based mutations with tester.run/apply patterns; adjusted GraphQL and HTTP request execution to flow through tester abstraction.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided; the PR is missing essential information about objectives, changes, and testing. Add a comprehensive description explaining the refactoring goals, affected test files, migration patterns, and testing performed. Include the Monday task link or reason for the PR.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main refactoring effort across multiple test files, focusing on the migration to new tester and operations patterns.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@test/file.e2e-spec.ts`:
- Around line 186-187: The test leaves a TODO about whether file names should
change and comments out the assertion comparing updatedFile.name and
initialFile.name; decide the intended behavior and codify it: if the new-version
import should rename files, re-enable and keep the assertion
expect(updatedFile.name).not.toEqual(initialFile.name) (or strengthen to an
exact expected name) using the variables updatedFile and initialFile; if names
must remain stable, change the test to assert equality
expect(updatedFile.name).toEqual(initialFile.name) and update the test
description accordingly; alternatively, if behavior is intentionally ambiguous,
add an explicit comment or a boolean flag (e.g., expectNameChange) and assert
based on that flag so the test no longer contains a lingering TODO.
- Around line 43-64: The function uploadFile is exported but only used within
this test file; remove the export so uploadFile becomes a local (non-exported)
function to comply with Biome's noExportsInTest rule—locate the uploadFile
declaration (the exported const uploadFile = (...) => async (tester: Tester) =>
{ ... }) and delete the leading "export" token; while here, review the TODO
around file naming on updates (line ~186) to ensure related helpers
(uploadFileContents, createFileVersion) handle names correctly.

In `@test/setup/gql-client/graphql-tester.ts`:
- Around line 80-86: The URL construction in createHttpClientForApp is wrong and
Got v14 needs prefixUrl for instance defaults: build the full GraphQL endpoint
using new URL('graphql', app.get(ConfigService).hostUrl$.value).toString() (so
the slash is handled) and pass that value as prefixUrl to got.extend({
prefixUrl: <that url>, enableUnixSockets: true }) so http.post() calls without a
URL work correctly; update the createHttpClientForApp function accordingly,
referencing createHttpClientForApp and the got.extend call.
🧹 Nitpick comments (2)
test/utility/create-app.ts (1)

2-39: Avoid reusing GraphQL auth hooks for raw uploads via tester.http

app.graphql.http includes auth hooks intended for GraphQL. When tester.http is used for presigned uploads (e.g., uploadFileContents), those headers may invalidate signatures. Consider a raw HTTP client for tester.http and keep auth only on GraphQL operations.

🔧 Possible adjustment
-import { createApp, type Tester } from '../setup';
+import { createApp, createHttpClientForApp, type Tester } from '../setup';
@@
-  app.tester = {
-    http: app.graphql.http,
+  const rawHttp = createHttpClientForApp(app);
+  app.tester = {
+    http: rawHttp,
     run: app.graphql.query,
     apply(op) {
       return op(app.tester);
     },
     app,
     legacyApp: app,
   };

Please verify with a presigned upload flow or the relevant e2e file-upload tests.

test/progress-report-media.e2e-spec.ts (1)

33-46: Consider migrating legacyApp usage to tester-bound helpers.
legacyApp is deprecated in the tester interface; once createProject/createLanguage have tester-based equivalents, switch to keep the refactor consistent.

Comment on lines +43 to +64
export const uploadFile =
(
parent: ID,
input: Partial<FakeFile> = {},
uploadRequest?: FileUploadRequested,
) =>
async (tester: Tester) => {
const { id, url } =
uploadRequest ?? (await tester.apply(requestFileUpload()));

const fakeFile = await tester.apply(uploadFileContents(url, input));

const fileNode = await tester.apply(
createFileVersion({
upload: id,
parent,
name: fakeFile.name,
}),
);

return fileNode;
}
return fileNode;
};
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all usages of uploadFile
echo "=== Searching for uploadFile usage ==="
rg -nP '\buploadFile\s*\(' --type typescript --type javascript

Repository: SeedCompany/cord-api-v3

Length of output: 157


🏁 Script executed:

#!/bin/bash
# Check if Biome is configured and look for noExportsInTest rule
echo "=== Looking for Biome configuration ==="
find . -maxdepth 2 -name "biome.json" -o -name "biome.jsonc" | head -20

echo "=== Checking for ESLint config ==="
find . -maxdepth 2 -name ".eslintrc*" -o -name "eslint.config.*" | head -20

echo "=== Looking at test file for TODO ==="
cat -n test/file.e2e-spec.ts | grep -i "todo"

Repository: SeedCompany/cord-api-v3

Length of output: 312


🏁 Script executed:

#!/bin/bash
# Check if test/operations/file.ts exists
echo "=== Checking for test/operations directory ==="
ls -la test/operations/ 2>/dev/null || echo "Directory does not exist"

# Also check the general test structure
echo "=== Test file structure ==="
find test -type f -name "*.ts" | head -20

Repository: SeedCompany/cord-api-v3

Length of output: 1186


🏁 Script executed:

#!/bin/bash
# Retry uploadFile search without type restriction
echo "=== Searching for uploadFile usage ==="
rg -n 'uploadFile'

Repository: SeedCompany/cord-api-v3

Length of output: 3001


🏁 Script executed:

#!/bin/bash
# Check biome.json for noExportsInTest rule
echo "=== Biome configuration ==="
cat biome.json

Repository: SeedCompany/cord-api-v3

Length of output: 1869


🏁 Script executed:

#!/bin/bash
# Look at the test file and operations/file.ts
echo "=== test/file.e2e-spec.ts content (lines 40-70) ==="
sed -n '40,70p' test/file.e2e-spec.ts

echo "=== test/operations/file.ts content ==="
cat -n test/operations/file.ts

Repository: SeedCompany/cord-api-v3

Length of output: 4959


🌐 Web query:

Biome noExportsInTest rule linter

💡 Result:

The Biome linter rule noExportsInTest forbids using export or module.exports inside test files (to avoid tests being run multiple times when a test file is imported). It’s recommended (enabled by default), has diagnostic id lint/suspicious/noExportsInTest, was added in v1.6.0, and its default severity is error.

Config example (biome.json):
{
"linter": {
"rules": {
"suspicious": {
"noExportsInTest": "error"
}
}
}
}

Invalid example:
export function myHelper() {}
describe('a test', () => { expect(1).toBe(1); });

Valid example:
function myHelper() {}
describe('a test', () => { expect(1).toBe(1); });

Sources: Biome rule docs and rules index. [1][2]

Sources
[1] Biome — noExportsInTest rule documentation.
[2] Biome — linter rules index.


Remove export from uploadFile (Biome noExportsInTest).

The uploadFile function is used only within this file, so it should be a local function, not exported. Biome forbids exports in test files to prevent unintended test execution.

Apply this change
-export const uploadFile =
+const uploadFile =

Also note the TODO at line 186 regarding file naming on updates.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const uploadFile =
(
parent: ID,
input: Partial<FakeFile> = {},
uploadRequest?: FileUploadRequested,
) =>
async (tester: Tester) => {
const { id, url } =
uploadRequest ?? (await tester.apply(requestFileUpload()));
const fakeFile = await tester.apply(uploadFileContents(url, input));
const fileNode = await tester.apply(
createFileVersion({
upload: id,
parent,
name: fakeFile.name,
}),
);
return fileNode;
}
return fileNode;
};
const uploadFile =
(
parent: ID,
input: Partial<FakeFile> = {},
uploadRequest?: FileUploadRequested,
) =>
async (tester: Tester) => {
const { id, url } =
uploadRequest ?? (await tester.apply(requestFileUpload()));
const fakeFile = await tester.apply(uploadFileContents(url, input));
const fileNode = await tester.apply(
createFileVersion({
upload: id,
parent,
name: fakeFile.name,
}),
);
return fileNode;
};
🧰 Tools
🪛 Biome (2.1.2)

[error] 42-64: Do not export from a test file.

(lint/suspicious/noExportsInTest)

🤖 Prompt for AI Agents
In `@test/file.e2e-spec.ts` around lines 43 - 64, The function uploadFile is
exported but only used within this test file; remove the export so uploadFile
becomes a local (non-exported) function to comply with Biome's noExportsInTest
rule—locate the uploadFile declaration (the exported const uploadFile = (...) =>
async (tester: Tester) => { ... }) and delete the leading "export" token; while
here, review the TODO around file naming on updates (line ~186) to ensure
related helpers (uploadFileContents, createFileVersion) handle names correctly.

Comment on lines +186 to +187
// TODO Files have their own names, should these be updated to match the new version's name?
// expect(updatedFile.name).not.toEqual(initialFile.name);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Resolve the TODO about file name updates.
Either codify the expected behavior in the test or track it explicitly so it doesn’t linger.

🤖 Prompt for AI Agents
In `@test/file.e2e-spec.ts` around lines 186 - 187, The test leaves a TODO about
whether file names should change and comments out the assertion comparing
updatedFile.name and initialFile.name; decide the intended behavior and codify
it: if the new-version import should rename files, re-enable and keep the
assertion expect(updatedFile.name).not.toEqual(initialFile.name) (or strengthen
to an exact expected name) using the variables updatedFile and initialFile; if
names must remain stable, change the test to assert equality
expect(updatedFile.name).toEqual(initialFile.name) and update the test
description accordingly; alternatively, if behavior is intentionally ambiguous,
add an explicit comment or a boolean flag (e.g., expectNameChange) and assert
based on that flag so the test no longer contains a lingering TODO.

Comment on lines +80 to +86
export const createHttpClientForApp = (app: TestApp) => {
const url = app.get(ConfigService).hostUrl$.value + 'graphql';
const http = got.extend({
url,
enableUnixSockets: true,
});
return http;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, find the file and understand its context
cd /tmp && find . -name "graphql-tester.ts" -type f 2>/dev/null | head -5

Repository: SeedCompany/cord-api-v3

Length of output: 49


🏁 Script executed:

# Search for hostUrl$ definition to understand its format
rg "hostUrl\$" -A 3 -B 3

Repository: SeedCompany/cord-api-v3

Length of output: 49


🏁 Script executed:

# Look at the full graphql-tester.ts file to understand usage
find . -name "graphql-tester.ts" -type f -exec wc -l {} \; 2>/dev/null

Repository: SeedCompany/cord-api-v3

Length of output: 111


🏁 Script executed:

# Check how createExecute() uses the HTTP client
rg "createExecute" -A 5 -B 2

Repository: SeedCompany/cord-api-v3

Length of output: 2651


🏁 Script executed:

# Search for Got documentation or version info
find . -name "package.json" -o -name "package-lock.json" | xargs grep -l "got" 2>/dev/null | head -3

Repository: SeedCompany/cord-api-v3

Length of output: 193


🏁 Script executed:

# Read the graphql-tester.ts file
cat -n test/setup/gql-client/graphql-tester.ts

Repository: SeedCompany/cord-api-v3

Length of output: 3032


🏁 Script executed:

# Read the gql-execute.ts file to see how http.post() is called
cat -n test/setup/gql-client/gql-execute.ts

Repository: SeedCompany/cord-api-v3

Length of output: 2692


🏁 Script executed:

# Check package.json for Got version
rg "\"got\"" package.json

Repository: SeedCompany/cord-api-v3

Length of output: 88


🏁 Script executed:

# Search for ConfigService and hostUrl$ definition
rg "hostUrl\$" --type ts -B 5 -A 5

Repository: SeedCompany/cord-api-v3

Length of output: 49


🏁 Script executed:

# Also check how hostUrl$ is set/used
rg "hostUrl\$\s*=" --type ts -B 3 -A 3

Repository: SeedCompany/cord-api-v3

Length of output: 49


🏁 Script executed:

# Search for ConfigService and hostUrl$ definition
rg "class ConfigService" -A 20 | head -40

Repository: SeedCompany/cord-api-v3

Length of output: 2633


🏁 Script executed:

# Look for hostUrl$ property definition
rg "hostUrl\$\s*:" --type ts -B 2 -A 2

Repository: SeedCompany/cord-api-v3

Length of output: 49


🏁 Script executed:

# Check test files to see what hostUrl$.value typically looks like
rg "hostUrl\$" test/ -B 3 -A 3

Repository: SeedCompany/cord-api-v3

Length of output: 49


🏁 Script executed:

# Search for ConfigService in the codebase more broadly
fd ConfigService --type ts | head -5

Repository: SeedCompany/cord-api-v3

Length of output: 236


🌐 Web query:

Got v14 default URL in got.extend() documentation and post() method behavior

💡 Result:

Short answer:

  • The instance default base URL option is prefixUrl (used with got.extend()), not "defaultUrl". [1]
  • got.post(...) (and instance.post/put/patch/...) simply sets options.method to the HTTP verb and performs the request. If you provide a body/json and options.method is not set, Got will set the method to POST. When using an instance created with got.extend({ prefixUrl: 'https://example.com' }), passing a relative path to instance.post('path') will be joined with prefixUrl. [1][2]

Sources:
[1] Got docs / README (instances, prefixUrl, got.extend).
[2] Got docs (convenience methods and body/json behavior).


🏁 Script executed:

# Check how 'url' option is used in got.extend() - search for examples
rg "got\.extend\(" -A 3 test/ | head -20

Repository: SeedCompany/cord-api-v3

Length of output: 298


🏁 Script executed:

# Look at the exact structure of hostUrl$ - is it a full URL with or without trailing slash?
cat -n src/core/config/config.service.ts | head -20

Repository: SeedCompany/cord-api-v3

Length of output: 1299


🌐 Web query:

Got v14 extend() url option vs prefixUrl - which parameter name is correct

💡 Result:

Use prefixUrl. In Got v14 the extend() base/instance URL option is named prefixUrl; the plain request URL is passed as the first argument (or as the url option). [1][2]

Sources:
[1] Got docs / package files (options.d.ts showing prefixUrl).
[2] Got npm docs describing prefixUrl and examples with got.extend().


🏁 Script executed:

# Check if 'url' option is valid in Got v14 or if it should be 'prefixUrl'
# Look for any Got type definitions or documentation in node_modules
find . -path ./node_modules/got -prune -o -name "*.d.ts" -type f -print 2>/dev/null | grep -i got | head -5

Repository: SeedCompany/cord-api-v3

Length of output: 49


🏁 Script executed:

# Check if there are any tests or examples that use the current code
rg "createHttpClientForApp" test/ -B 2 -A 5

Repository: SeedCompany/cord-api-v3

Length of output: 2100


🌐 Web query:

Got v14 url option vs prefixUrl in got.extend() - is url parameter deprecated or invalid

💡 Result:

Short answer: The url option is still valid in got v14 — it’s the per-request URL. prefixUrl is a separate option intended for setting a base when creating instances via got.extend(), and will be prepended to the request url (with some rules such as being ignored when the request url is a URL instance and disallowing leading slashes). prefixUrl is not a replacement that makes url deprecated. [1][2][3]

Relevant docs:

  • prefixUrl behavior and got.extend() usage. [1]
  • Type definitions/notes in got v14 source (shows prefixUrl docs and rules). [2]
  • npm/jsdocs summary showing prefixUrl guidance (useful for creating instances). [3]

If you want, tell me the exact code you’re using and I’ll point out why prefixUrl/url behave that way in your case.


🏁 Script executed:

# Look at the actual test failures or usages to see if the current code works
rg "got.extend\(" -A 5 -B 1

Repository: SeedCompany/cord-api-v3

Length of output: 501


🏁 Script executed:

# Check what env.url() returns and how it formats URLs
rg "env\.url\(" -B 2 -A 2 | head -30

Repository: SeedCompany/cord-api-v3

Length of output: 728


🏁 Script executed:

# Look for tests that might show the actual hostUrl$ value
rg "hostUrl" test/ -B 2 -A 2

Repository: SeedCompany/cord-api-v3

Length of output: 711


🏁 Script executed:

# Check if there are any tests running against this code
find test/ -name "*.spec.ts" -o -name "*.test.ts" | xargs grep -l "createHttpClientForApp\|createTester" 2>/dev/null | head -3

Repository: SeedCompany/cord-api-v3

Length of output: 49


🏁 Script executed:

# Try to understand if the code currently works by checking Git history or any error logs
git log --oneline test/setup/gql-client/graphql-tester.ts | head -5

Repository: SeedCompany/cord-api-v3

Length of output: 145


🏁 Script executed:

# Double-check the exact value structure - let's look at what URL instance toString() returns
python3 << 'EOF'
from urllib.parse import urljoin

# Simulate what happens with current code
hostUrl_value = "http://localhost:3000"
result = hostUrl_value + 'graphql'
print(f"Current code result: {result}")
print(f"Is it valid? {'Yes' if result.startswith('http') else 'No'}")

# What the suggested fix would do
import urllib.parse
url_obj = urllib.parse.urljoin(hostUrl_value + '/', 'graphql')
print(f"\nWith trailing slash fix: {url_obj}")
EOF

Repository: SeedCompany/cord-api-v3

Length of output: 153


🏁 Script executed:

# Check if there are any integration tests that would catch this
rg "\.post\(" test/setup/gql-client/ -B 2 -A 3

Repository: SeedCompany/cord-api-v3

Length of output: 421


🏁 Script executed:

# Verify the actual URL construction with the new URL API
cat > /tmp/test_url.mjs << 'EOF'
const hostUrl = "http://localhost:3000";
const url = new URL('graphql', hostUrl).toString();
console.log("Result:", url);

// What the current code does
const current = hostUrl + 'graphql';
console.log("Current code:", current);
EOF
node /tmp/test_url.mjs

Repository: SeedCompany/cord-api-v3

Length of output: 147


🏁 Script executed:

# Verify how hostUrl$.value is being set in tests
cat -n test/setup/create-app.ts | grep -A 5 -B 5 "hostUrl\$"

Repository: SeedCompany/cord-api-v3

Length of output: 49


🏁 Script executed:

# Check if there are any URL instances being passed to hostUrl$
rg "new URL" test/setup/ -B 2 -A 2

Repository: SeedCompany/cord-api-v3

Length of output: 347


🌐 Web query:

Got v14 got.extend({ url: '...' }) and instance.post() without URL parameter - does it work or require prefixUrl

💡 Result:

Short answer: It requires prefixUrl. got.extend({ url: '...' }) does not set a base for calls that omit the URL — use prefixUrl in the defaults (got.extend({ prefixUrl: 'https://api.example.com' })) or pass a URL to instance.post(). [1][2]

Sources:

  • got options docs (prefixUrl behavior and example). [1]
  • got extend / instance usage (examples showing prefixUrl with got.extend()). [2]

Fix URL construction and use prefixUrl for Got v14 instance default

Two issues:

  1. Malformed URL: hostUrl$.value + 'graphql' produces http://localhost:3000graphql (missing /). Use new URL('graphql', hostUrl$.value).toString() instead.

  2. Got v14 requires prefixUrl: The url option in got.extend() does not set a default base URL for instance method calls. Since createExecute() calls http.post({...}) without a URL argument, you must use prefixUrl instead of url. Change:

-  const url = app.get(ConfigService).hostUrl$.value + 'graphql';
-  const http = got.extend({
-    url,
+  const http = got.extend({
+    prefixUrl: new URL('graphql', app.get(ConfigService).hostUrl$.value).toString(),
     enableUnixSockets: true,
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export const createHttpClientForApp = (app: TestApp) => {
const url = app.get(ConfigService).hostUrl$.value + 'graphql';
const http = got.extend({
url,
enableUnixSockets: true,
});
return http;
export const createHttpClientForApp = (app: TestApp) => {
const http = got.extend({
prefixUrl: new URL('graphql', app.get(ConfigService).hostUrl$.value).toString(),
enableUnixSockets: true,
});
return http;
🤖 Prompt for AI Agents
In `@test/setup/gql-client/graphql-tester.ts` around lines 80 - 86, The URL
construction in createHttpClientForApp is wrong and Got v14 needs prefixUrl for
instance defaults: build the full GraphQL endpoint using new URL('graphql',
app.get(ConfigService).hostUrl$.value).toString() (so the slash is handled) and
pass that value as prefixUrl to got.extend({ prefixUrl: <that url>,
enableUnixSockets: true }) so http.post() calls without a URL work correctly;
update the createHttpClientForApp function accordingly, referencing
createHttpClientForApp and the got.extend call.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors E2E tests for file and progress report media operations to use the new Tester pattern and operation-based approach. The refactoring extracts file and directory operations into reusable modules (test/operations/file.ts and test/operations/directory.ts), replacing imperative test code with composable operations that follow a consistent curried pattern.

Changes:

  • Extracted file operations (requestFileUpload, uploadFileContents, createFileVersion, etc.) to test/operations/file.ts using the operation pattern
  • Created new test/operations/directory.ts with directory operations (createRootDirectory, createDirectory)
  • Refactored test infrastructure to expose http client on Tester and GraphQLTestClient interfaces
  • Updated progress-report-media.e2e-spec.ts and file.e2e-spec.ts to use new Tester pattern with createTesterWithRole and operations
  • Added identity property to root tester and IdentifiedTester type
  • Migrated from lodash/luxon utilities to internal @seedcompany/common equivalents (DurationIn, Case.capital)

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
test/utility/index.ts Removed export for deleted create-directory module
test/utility/create-graphql-client.ts Added http property to GraphQLTestClient, refactored to use shared HTTP client creation
test/utility/create-directory.ts Deleted file, functionality moved to test/operations/directory.ts
test/utility/create-app.ts Added tester property to deprecated TestApp for gradual migration
test/setup/gql-client/root-tester.ts Added identity property to root tester containing logged-in user info
test/setup/gql-client/graphql-tester.ts Added http property, extracted createHttpClientForApp helper function
test/setup/gql-client/gql-execute.ts Refactored to accept Got instance instead of options object
test/operations/directory.ts New file with directory operations using operation pattern
test/operations/file.ts Refactored file operations to use curried operation pattern with Tester
test/progress-report-media.e2e-spec.ts Complete refactoring to use new Tester pattern, IdentifiedTester, and file operations
test/file.e2e-spec.ts Extensive refactoring to use Tester pattern, replaced session management with testers
test/engagement.e2e-spec.ts Minor updates to use new file operations via app.tester.apply()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -236,51 +218,51 @@ describe('File e2e', () => {

it('create directory', async () => {
const name = startCase(faker.lorem.words());
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using Case.capital from @seedcompany/common/case instead of lodash's startCase for consistency with the new directory operations pattern. The newly created test/operations/directory.ts uses Case.capital for name generation, and following this pattern would align with the codebase convention of preferring internal library utilities.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants