Skip to content

Commit

Permalink
File Explorer: Add e2e tests (#4827)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
ericpgreen2 and AdityaHegde authored May 9, 2024
1 parent 02515c3 commit c43d62d
Show file tree
Hide file tree
Showing 4 changed files with 158 additions and 28 deletions.
77 changes: 53 additions & 24 deletions runtime/drivers/file/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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(),
}
}

Expand Down
10 changes: 6 additions & 4 deletions web-common/src/features/file-explorer/NavDirectory.svelte
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<script lang="ts">
import NavDirectoryEntry from "@rilldata/web-common/features/file-explorer/NavDirectoryEntry.svelte";
import {
NavDragData,
navEntryDragDropStore,
} from "@rilldata/web-common/features/file-explorer/nav-entry-drag-drop-store";
import NavDirectoryEntry from "@rilldata/web-common/features/file-explorer/NavDirectoryEntry.svelte";
import NavFile from "./NavFile.svelte";
import { directoryState } from "./directory-store";
import type { Directory } from "./transform-file-list";
Expand All @@ -24,12 +24,14 @@
$dragData && $dropDirs[$dropDirs.length - 1] === directory.path;
</script>

<div
<ul
id={`nav-${directory.path}`}
aria-label={directory.path}
role="directory"
class="w-full"
class:bg-slate-100={isDragDropHover}
on:mouseenter={() => navEntryDragDropStore.onMouseEnter(directory.path)}
on:mouseleave={() => navEntryDragDropStore.onMouseLeave()}
role="directory"
>
{#if directory.path !== "/"}
<NavDirectoryEntry dir={directory} {onDelete} {onMouseDown} {onRename} />
Expand Down Expand Up @@ -61,4 +63,4 @@
/>
{/each}
{/if}
</div>
</ul>
Original file line number Diff line number Diff line change
Expand Up @@ -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}
>
<CaretDownIcon
className="flex-none text-gray-400 {expanded ? '' : 'transform -rotate-90'}"
Expand Down
97 changes: 97 additions & 0 deletions web-local/tests/file-explorer.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import { expect } from "playwright/test";
import { test } from "./utils/test";

test.describe("File Explorer", () => {
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();
});
});
});

1 comment on commit c43d62d

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

Please sign in to comment.