Added Puppeteer UI interaction tests by LanieOkorodudu · Pull Request #1713 · UniversalViewer/universalviewer

LanieOkorodudu · 2026-03-23T15:28:32Z

Description of what you did:

Added Puppeteer tests covering basic UI interactions:

next/previous image navigation
zoom in/out
rotate image
opening and closing adjust image control

For now the new test are kept simple and focused on basic interactions to align with the existing test structure, with plans to add more test later.

vercel · 2026-03-23T15:28:42Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
universalviewer	Ready	Preview, Comment	Apr 7, 2026 5:32pm

vercel · 2026-03-23T15:28:45Z

@LanieOkorodudu is attempting to deploy a commit to the Universal Viewer Team on Vercel.

A member of the Team first needs to authorize it.

demiankatz

Thanks, @LanieOkorodudu, this is off to a good start! I think it would be beneficial to add some more specific assertions to more strongly confirm that the tests are doing what is expected. Obviously there are limits to what we can do without visual inspection, but see below for some ideas of additional details we could measure to strengthen the tests.

I'm not an expert with Jest or Puppeteer, so I apologize if any of my suggestions are off-base. I'm happy to do further research and discuss in more detail as needed!

demiankatz · 2026-03-24T11:02:55Z

  describe("thumb cache invalidation", () => {
    beforeEach(async () => {
-      await page.goto("http://localhost:4444/");
+      await page.goto("http://localhost:4444/"); // //update this side to your own local host when manually testing in your local machine


Don't need double comments:

Suggested change

await page.goto("http://localhost:4444/"); // //update this side to your own local host when manually testing in your local machine

await page.goto("http://localhost:4444/"); // update this side to your own local host when manually testing in your local machine

I wonder if it would make sense to define a file that we could put in .gitignore to contain the base URL of the test suite. The tests could check for that file and read it if it exists to get the URL, but default to localhost:4444 if it's missing. Then we wouldn't need to reconfigure multiple test files to change the target URL.

(This could also potentially be done with an environment variable -- maybe that would be even better, though I don't have a strong opinion).

LanieOkorodudu · 2026-03-24T11:28:21Z

Thanks @demiankatz for taking time to look into this. I thought of adding specific assertion but I was hesitating to complicate things, but it's good you mentioned, I will implement your suggestion and I believe this will improve the test.

LanieOkorodudu · 2026-03-28T11:48:25Z

@demiankatz, I added a reset after each test to bring the viewer back to the main URL, since the tests change the viewer state (navigation, zoom, rotation, etc.).

This way each test starts from a clean state and doesn’t get affected by what happened in the previous one.

Not sure if this is the best approach, so I’d appreciate your feedback. Thanks

demiankatz

Thanks for the progress here, @LanieOkorodudu, this is coming along very well!

See below for a few comments, many to do with minor style issues. If you need help investigating the related tooling, please let me know and I'll be happy to help as time permits!

LanieOkorodudu · 2026-03-30T13:31:40Z

Thanks for the progress here, @LanieOkorodudu, this is coming along very well!

See below for a few comments, many to do with minor style issues. If you need help investigating the related tooling, please let me know and I'll be happy to help as time permits!

Thanks for the feedback, really appreciate it! I’ll dig into the tooling and let you know if I get stuck.

LanieOkorodudu · 2026-04-01T12:44:09Z

@demiankatz, I’ve implemented your suggestions for the next/previous navigation, zoom, rotation, and adjust image tests.

I also fixed the indentation issues and ran Prettier, hopefully everything is aligned properly now.
The next/previous tests now check the actual page values and confirm they change by 1 (with a small adjustment so it works when starting on the first page).
The zoom test now checks the xywh value instead of the full URL.
I also applied your suggestion for the rotation check and updated the adjust image test name.
let me know if I may have missed anything.

demiankatz

Thanks, @LanieOkorodudu, this is shaping up nicely. See below for a few comments, all along very similar lines.

A couple of other general things:

1.) We didn't complete the conversation above about a standard way to set/override the URL of the instance under test. That's not a problem that we need to solve here, though -- I'm happy to set it aside and figure it out in a separate PR later.

2.) I wonder if it's also adding assertions about the previous button being disabled on the first page -- while we're testing going forward and back, maybe some extra checks about the button enabling/disabling as appropriate would be worth adding for completeness.

LanieOkorodudu · 2026-04-07T17:42:50Z

Thanks, @LanieOkorodudu, this is shaping up nicely. See below for a few comments, all along very similar lines.

A couple of other general things:

1.) We didn't complete the conversation above about a standard way to set/override the URL of the instance under test. That's not a problem that we need to solve here, though -- I'm happy to set it aside and figure it out in a separate PR later.

2.) I wonder if it's also adding assertions about the previous button being disabled on the first page -- while we're testing going forward and back, maybe some extra checks about the button enabling/disabling as appropriate would be worth adding for completeness.

Thanks for the feedback @demiankatz,

I’ve intentionally left the URL override as-is for now, and yes I’ll address that in a separate PR.
I’ve added some checks for the previous button so it’s disabled on the first page, enabled after going forward, and disabled again when returning.

I’ve also removed the null checks and made the assertions more specific.
For the cv values, I kept the expectations as 0 → 1 based on what I observed earlier, since it seems to be zero-based in the URL while the viewer shows page numbers starting from 1.

I also updated the rotation test to read the rotation from the viewer state instead of the URL, since the URL didn’t seem to reflect the rotation reliably.

Let me know if these changes address it or if I've made anything worse.

demiankatz

Thanks for the progress here, @LanieOkorodudu! I think this is almost ready to merge. See below for a few minor suggestions.

demiankatz · 2026-04-16T18:06:06Z

+    // navigate to next image
+    it("can navigate to next image", async () => {
+      await page.waitForSelector(".btn.imageBtn.next", { visible: true });
+
+      const urlBefore = page.url();
+      const before = getCanvasValue(urlBefore);
+
+      await page.click(".btn.imageBtn.next");
+      await waitForCanvasValue(page, 1);
+
+      const urlAfter = page.url();
+      const after = getCanvasValue(urlAfter);
+
+      expect(before).toBe(0);
+      expect(after).toBe(1);
+    });


Do we need this test? It seems to be mostly redundant with the following one. Should we rename that from "can navigate to previous image" to "can navigate back and forth" and then delete this first test?

demiankatz · 2026-04-16T18:09:06Z

+      });
+
+      const initialRot = await getRotationFromNavigator();
+      const initialTransform = await page.evaluate(() => {


Is there any reason not to use getRotationFromNavigator instead of directly accessing the .displayregioncontainer transform style in multiple places? I imagine it's possible that a future OpenSeadragon release might change the way this works, so having it centralized in a single support method would make it easier to fix if something breaks it in the future. Of course, if there's a reason you need "raw" access to the value, I can live with it as-is. :-)

demiankatz · 2026-04-16T18:11:02Z

+      await page.evaluate((selector) => {
+        const el = document.querySelector(selector);
+        if (el) el.click();
+      }, closeBtn);


I'm not sure I understand why evaluate is being used here. Can this be simplified? If nothing else, maybe this would be less confusing (if I understand the syntax correctly):

Suggested change

await page.evaluate((selector) => {

const el = document.querySelector(selector);

if (el) el.click();

}, closeBtn);

await page.evaluate(() => {

const el = document.querySelector(closeBtn);

if (el) el.click();

});

Added Puppeteer UI interaction tests

8a34823

vercel bot deployed to Preview – universalviewer March 23, 2026 15:29 View deployment

LanieOkorodudu requested a review from demiankatz March 23, 2026 15:32

switch back local host to 4444

3c11aed

vercel bot deployed to Preview – universalviewer March 23, 2026 15:46 View deployment

demiankatz requested changes Mar 24, 2026

View reviewed changes

added assertions

edfcacc

vercel bot deployed to Preview – universalviewer March 28, 2026 11:37 View deployment

remove undeclared helper

f005cb5

vercel bot deployed to Preview – universalviewer March 28, 2026 11:46 View deployment

demiankatz requested changes Mar 30, 2026

View reviewed changes

Improve test assertions and formatting

c5e3650

vercel bot deployed to Preview – universalviewer April 1, 2026 12:41 View deployment

demiankatz requested changes Apr 2, 2026

View reviewed changes

Comment thread __tests__/test.js Outdated

Comment thread __tests__/test.js Outdated

Comment thread __tests__/test.js Outdated

remove null checks

2e59cc6

vercel bot deployed to Preview – universalviewer April 7, 2026 17:32 View deployment

demiankatz requested changes Apr 16, 2026

View reviewed changes

	await page.goto("http://localhost:4444/"); // //update this side to your own local host when manually testing in your local machine
	await page.goto("http://localhost:4444/"); // update this side to your own local host when manually testing in your local machine

Uh oh!

Conversation

LanieOkorodudu commented Mar 23, 2026

Description of what you did:

Uh oh!

vercel bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Mar 23, 2026

Uh oh!

demiankatz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

demiankatz Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LanieOkorodudu commented Mar 24, 2026

Uh oh!

LanieOkorodudu commented Mar 28, 2026

Uh oh!

demiankatz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LanieOkorodudu commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LanieOkorodudu commented Apr 1, 2026

Uh oh!

demiankatz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LanieOkorodudu commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

demiankatz left a comment

Choose a reason for hiding this comment

Uh oh!

demiankatz Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

demiankatz Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

demiankatz Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vercel bot commented Mar 23, 2026 •

edited

Loading

LanieOkorodudu commented Mar 30, 2026 •

edited

Loading

LanieOkorodudu commented Apr 7, 2026 •

edited

Loading