Backend code for quitOnClose option

[adamkpickering]

Closes #3261.

[mook-as]

There's a few lint issues found by CI as well.

[mook-as]

I assume we're waiting for the shutdown to complete? Since the CI machines are randomly slow, it would be good if there's something we could actually poll for (which would let us have a much longer maximum wait time, and finish much sooner on average).

[adamkpickering]

I've tried to get something like this working, but haven't been able to. The way I've tried is to listen to the quit event on Electron.app, and set a variable when the event fires:

  test('should quit when quitOnClose is true and window is closed', async() => {
    createDefaultSettings({ window: { quitOnClose: true } });
    const {electronApp, page} = await startRancherDesktop(__filename);
    const browserWindowHandle = await electronApp.browserWindow(page);

    let hasQuit = false;
    electronApp.evaluate(({app}) => {
      app.on('quit', () => {
        console.log('quit event occurred')
        hasQuit = true;
      });
    });

    browserWindowHandle.evaluate((browserWindow: Electron.BrowserWindow) => browserWindow.close());

    const sleepTime = 5000;
    const iterCount = 6;
    for (let i = 0; i < iterCount; i++) {
      console.log(`hasQuit: ${ hasQuit }`)
      if (hasQuit) {
        return;
      }
      await sleep(sleepTime)
    }
    // fail the test if RD has not quit by now
    expect(true).toBe(false);
  });

But it doesn't seem to work. I'm not very confident with playwright though. Maybe you can see something I'm doing wrong? Or maybe you're away of another way of doing it? I can't think of anything else...

[mook-as]

I believe .evaluate works something like serializing the function to text, and eval() it on the other side. So in your case, hasQuit isn't getting a closure, so it's never set to true on the test side.

In my case, I do everything within evaluate(), and it only returns a Promise<boolean> which the documentation says it can.

[ericpromislow]

I can't get the e2e test to start running on both linux and mac

[mook-as]

Works fine for me on mac & Windows, and CI (running Linux) look fine with it too.

[mook-as]

Shouldn't we call await electronApp.context.tracing.start({ screenshots: true, snapshots: true }); somewhere in this function?

[mook-as]

This works fine, but has a minimum run time of 30s (for the case where we don't want to quit).

Possible alternate, but I'm not sure I've got enough of a wait in:

function closeAllWindows(electronApp: ElectronApplication): Promise<boolean> {
  return electronApp.evaluate(async({ app, BrowserWindow }) => {
    const quitReady = new Promise<boolean>((resolve) => {
      app.on('will-quit', () => resolve(true));
      app.on('window-all-closed', () => {
        setTimeout(() => resolve(false), 1_000);
      });
    });

    await Promise.all(BrowserWindow.getAllWindows().map((window) => {
      return new Promise<void>((resolve) => {
        window.on('closed', resolve);
        window.close();
      });
    }));

    return await quitReady;
  });
}

…
  test('should quit when quitOnClose is true and window is closed', async() => {
    createDefaultSettings({ window: { quitOnClose: true } });
    const { electronApp } = await startRancherDesktop(__filename);

    await expect(closeAllWindows(electronApp)).resolves.toBe(true);
  });

[adamkpickering]

That's a better way to do it. I have to say though, I'm confused as to why my use of electronApp.evaluate above doesn't work, whereas its use here does. Can you not refer to a variable from an outer scope or something?

[mook-as]

See above — yeah, I think evaluate() is really serialize-and-eval(), so closures wouldn't work. This is why in my implementation all of the work happens inside the evaluate() call (in the Electron process), and only a boolean is returned to the test process.

[mook-as]

Pretty happy with it, just some nits.

[mook-as]

FWIW, ElectronApplication and Page can be changed to an import type, but that's mostly just moving deck chairs.

[adamkpickering]

If it's alright, I'd like to keep it as it is. The following link is convincing to me: microsoft/TypeScript#39861

[ericpromislow]

e2e tests pass now on my machines