From c43d62d49cc3916ba10da3549424c3f7d830067c Mon Sep 17 00:00:00 2001 From: Eric P Green Date: Thu, 9 May 2024 07:33:37 -0700 Subject: [PATCH] File Explorer: Add e2e tests (#4827) * Better HTML semantics and accessibility * Add a couple e2e tests * Try new locator * Try hovering before clicking * Try `waitForSelector` * Try waiting for API response * Remove `waitForSelector` * Try asserting `GetFiles` response * Parallelise click and request check * Add debug * Fix file watcher for linux * More logs * Add renamed directories to watcher * Cleanup logs * Tweak checks for performance * Additional debugging * Add dir handling in flush * Cleanup * Use `keyboard.type` rather than `fill` * Don't use autosave * Remove devcontainer * PR comments --------- Co-authored-by: Aditya Hegde --- runtime/drivers/file/watcher.go | 77 ++++++++++----- .../file-explorer/NavDirectory.svelte | 10 +- .../file-explorer/NavDirectoryEntry.svelte | 2 + web-local/tests/file-explorer.spec.ts | 97 +++++++++++++++++++ 4 files changed, 158 insertions(+), 28 deletions(-) create mode 100644 web-local/tests/file-explorer.spec.ts diff --git a/runtime/drivers/file/watcher.go b/runtime/drivers/file/watcher.go index 0d55118ccc4..c27a082cea7 100644 --- a/runtime/drivers/file/watcher.go +++ b/runtime/drivers/file/watcher.go @@ -34,7 +34,15 @@ type watcher struct { mu sync.Mutex subscribers map[int]drivers.WatchCallback nextSubscriberID int - buffer map[string]drivers.WatchEvent + buffer map[string]watchEvent +} + +type watchEvent struct { + eventType runtimev1.FileEvent + path string + relPath string + dir bool + isCreate bool } // newWatcher creates a new watcher for the given root directory. @@ -52,7 +60,7 @@ func newWatcher(root string, ignorePaths []string, logger *zap.Logger) (*watcher watcher: fsw, done: make(chan struct{}), subscribers: make(map[int]drivers.WatchCallback), - buffer: make(map[string]drivers.WatchEvent), + buffer: make(map[string]watchEvent), } err = w.addDir(root, false, true) @@ -119,16 +127,42 @@ func (w *watcher) flush() { return } + for p, event := range w.buffer { + if !event.isCreate { + continue + } + // check for directory for CREATE events + info, err := os.Stat(event.path) + event.dir = err == nil && info.IsDir() + if event.dir { + // add directory to tracking paths + err = w.addDir(event.path, true, false) + if err != nil { + delete(w.buffer, p) + continue + } + } + w.buffer[p] = event + } + events := maps.Values(w.buffer) + driverEvents := make([]drivers.WatchEvent, len(events)) + for i, event := range events { + driverEvents[i] = drivers.WatchEvent{ + Type: event.eventType, + Path: event.relPath, + Dir: event.dir, + } + } w.mu.Lock() defer w.mu.Unlock() for _, fn := range w.subscribers { - fn(events) + fn(driverEvents) } - w.buffer = make(map[string]drivers.WatchEvent) + w.buffer = make(map[string]watchEvent) } func (w *watcher) run() { @@ -158,14 +192,15 @@ func (w *watcher) runInner() error { return nil } - we := drivers.WatchEvent{} + we := watchEvent{} if e.Has(fsnotify.Remove) || e.Has(fsnotify.Rename) { - we.Type = runtimev1.FileEvent_FILE_EVENT_DELETE + we.eventType = runtimev1.FileEvent_FILE_EVENT_DELETE } else if e.Has(fsnotify.Create) || e.Has(fsnotify.Write) || e.Has(fsnotify.Chmod) { - we.Type = runtimev1.FileEvent_FILE_EVENT_WRITE + we.eventType = runtimev1.FileEvent_FILE_EVENT_WRITE } else { continue } + we.isCreate = e.Has(fsnotify.Create) path, err := filepath.Rel(w.root, e.Name) if err != nil { @@ -174,28 +209,21 @@ func (w *watcher) runInner() error { } path = filepath.Join("/", path) - we.Path = path + we.relPath = path + we.path = e.Name // Do not send files for ignored paths if drivers.IsIgnored(path, w.ignorePaths) { continue } - if e.Has(fsnotify.Create) { - info, err := os.Stat(e.Name) - we.Dir = err == nil && info.IsDir() + existing, ok := w.buffer[path] + if ok && existing.isCreate && we.eventType == runtimev1.FileEvent_FILE_EVENT_WRITE { + // copy over `IsCreate` within the batch for a path + we.isCreate = existing.isCreate } - w.buffer[path] = we - // Calling addDir after appending to w.buffer, to sequence events correctly - if we.Dir && e.Has(fsnotify.Create) { - err = w.addDir(e.Name, true, false) - if err != nil { - return err - } - } - // Reset the timer so we only flush when no events have been observed for batchInterval. // (But to avoid the buffer growing infinitely in edge cases, we enforce a max buffer size.) if len(w.buffer) < maxBufferSize { @@ -236,10 +264,11 @@ func (w *watcher) addDir(path string, replay, errIfNotExist bool) error { } ep = filepath.Join("/", ep) - w.buffer[ep] = drivers.WatchEvent{ - Path: ep, - Type: runtimev1.FileEvent_FILE_EVENT_WRITE, - Dir: e.IsDir(), + w.buffer[ep] = watchEvent{ + path: fullPath, + relPath: ep, + eventType: runtimev1.FileEvent_FILE_EVENT_WRITE, + dir: e.IsDir(), } } diff --git a/web-common/src/features/file-explorer/NavDirectory.svelte b/web-common/src/features/file-explorer/NavDirectory.svelte index 2933b528329..736a85306e7 100644 --- a/web-common/src/features/file-explorer/NavDirectory.svelte +++ b/web-common/src/features/file-explorer/NavDirectory.svelte @@ -1,9 +1,9 @@ -
navEntryDragDropStore.onMouseEnter(directory.path)} on:mouseleave={() => navEntryDragDropStore.onMouseLeave()} - role="directory" > {#if directory.path !== "/"} @@ -61,4 +63,4 @@ /> {/each} {/if} -
+ diff --git a/web-common/src/features/file-explorer/NavDirectoryEntry.svelte b/web-common/src/features/file-explorer/NavDirectoryEntry.svelte index 3c25adf1b9d..311182af670 100644 --- a/web-common/src/features/file-explorer/NavDirectoryEntry.svelte +++ b/web-common/src/features/file-explorer/NavDirectoryEntry.svelte @@ -81,6 +81,8 @@ on:click={() => toggleDirectory(dir)} on:mousedown={(e) => onMouseDown(e, { id, filePath: dir.path, isDir: true })} style:padding-left="{padding}px" + aria-controls={`nav-${dir.path}`} + aria-expanded={expanded} > { + test.describe("File CRUD Operations", () => { + test("should create, rename, edit, and delete a file", async ({ page }) => { + // Create a new file + await page.getByLabel("Add Asset").click(); + await page.getByRole("menuitem", { name: "More" }).hover(); + await page.getByRole("menuitem", { name: "Blank file" }).click(); + await expect( + page.getByRole("link", { name: "untitled_file" }), + ).toBeVisible(); + await expect( + page.getByLabel("untitled_file", { exact: true }), + ).toBeVisible(); + + // Rename the file + await page.getByLabel("/untitled_file actions menu").click(); + await page.getByRole("menuitem", { name: "Rename..." }).click(); + await page.getByLabel("File name").click(); + await page.getByLabel("File name").press("Meta+a"); + await page.getByLabel("File name").fill("README.md"); + await page.getByLabel("File name").press("Enter"); + await expect(page.getByRole("link", { name: "README.md" })).toBeVisible(); + + // Edit the file + await page.getByLabel("Auto-save").click(); // Turn off auto-save + await page.getByRole("textbox").nth(1).click(); + await page.keyboard.type("Here's a README.md file for the e2e test!"); + await page.getByRole("button", { name: "Save" }).click(); + // Wait half a second for the changes to be saved + await page.waitForTimeout(500); + // Navigate away from the file and back to it to verify the changes + await page.getByRole("link", { name: "rill.yaml" }).click(); + await page.getByRole("link", { name: "README.md" }).click(); + await expect( + page.getByText("Here's a README.md file for the e2e test!"), + ).toBeVisible(); + + // Delete the file + await page.getByLabel("/README.md actions menu").click(); + await page.getByRole("menuitem", { name: "Delete" }).click(); + await expect( + page.getByRole("link", { name: "README.md" }), + ).not.toBeVisible(); + }); + }); + + test.describe("Folder CRUD Operations", () => { + test("should create, rename, add sub-folder, and delete the folder", async ({ + page, + }) => { + // Create a new folder + await page.getByLabel("Add Asset").click(); + await page.getByRole("menuitem", { name: "More" }).hover(); + await page.getByRole("menuitem", { name: "Folder" }).click(); + await expect( + page.getByRole("directory", { name: "untitled_folder" }), + ).toBeVisible(); + + // Rename the folder + await page.getByRole("directory", { name: "untitled_folder" }).hover(); + await page.getByLabel("untitled_folder actions menu").click(); + await page.getByRole("menuitem", { name: "Rename..." }).click(); + await page.getByLabel("Folder name").click(); + await page.getByLabel("Folder name").press("Meta+a"); + await page.getByLabel("Folder name").fill("my-directory"); + await page.getByLabel("Folder name").press("Enter"); + + // Add something to the folder + await page.getByRole("directory", { name: "my-directory" }).hover(); + await page.getByLabel("my-directory actions menu").click(); + await page.getByRole("menuitem", { name: "New folder" }).hover(); + const [createDirectoryResponse, getFilesResponse] = await Promise.all([ + page.waitForResponse("**/v1/instances/default/files/dir"), + page.waitForResponse("**/v1/instances/default/files"), + page.getByRole("menuitem", { name: "New folder" }).click(), + ]); + + expect(createDirectoryResponse.status()).toBe(200); + expect(getFilesResponse.status()).toBe(200); + const resp = await getFilesResponse.json(); + expect(resp.files.length).toBe(4); + await expect( + page.getByRole("directory", { + name: "my-directory/untitled_folder", + }), + ).toBeVisible(); + + // Delete the folder + await page.getByLabel("my-directory actions menu").click(); + await page.getByRole("menuitem", { name: "Delete" }).click(); + await page.getByRole("button", { name: "Delete" }).click(); + }); + }); +});