Skip to content

Qa 83/login logout of vortex#22898

Draft
RedRanger14 wants to merge 2 commits into
masterfrom
QA-83/Login-logout-of-vortex
Draft

Qa 83/login logout of vortex#22898
RedRanger14 wants to merge 2 commits into
masterfrom
QA-83/Login-logout-of-vortex

Conversation

@RedRanger14
Copy link
Copy Markdown
Contributor

Adding a logout test for Vortex

Added premium user for Vortex

Because all tests need to be independent from one another. I have created a vortex-app-isolated fixture that launches a fresh Electron app and fresh user-data directory per test.

RedRanger14 and others added 2 commits April 29, 2026 16:11
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
@RedRanger14 RedRanger14 self-assigned this Apr 29, 2026
* Removes ELECTRON_RUN_AS_NODE (set by pnpm) and isolates user data.
*/
function buildElectronEnv(userDataDir: string): Record<string, string> {
const env: Record<string, string> = {};
Copy link
Copy Markdown
Contributor

@IDCs IDCs May 6, 2026

Choose a reason for hiding this comment

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

As far as I can see, this is copied verbatim (+/- a few comments) from vortex-app.ts. There are currently ~150 lines of duplication between the two fixtures, lets extract those functions into their own file and import them into the fixtures to avoid divergence between the two.

As far as I can see the duplicated functions are: resolveMainDir, resolveElectronBinary, buildElectronEnv, waitForMainWindow

await use(mainWindow);
return;
} catch (error) {
if (!mainWindow.isClosed()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So although your code avoids this at runtime, calling use twice within the same fixture is violating the playwright fixture contract. This is potential for undefined behaviour. Additionally we're potentially dropping relevant errors from the first use call.

I suggest something like:

vortexWindow: async ({ vortexApp }, use) => {
  const waitForReady = async (win: Page) => {
    await win.waitForLoadState("domcontentloaded");
    await win.waitForFunction(
      () => (document.body?.innerText?.length ?? 0) > 0,
      { timeout: 60_000 },
    );
  };

  let mainWindow = await waitForMainWindow(vortexApp);
  try {
    await waitForReady(mainWindow);
  } catch (error) {
    if (!mainWindow.isClosed()) throw error;
    mainWindow = await waitForMainWindow(vortexApp);
    await waitForReady(mainWindow);
  }

  await use(mainWindow);
},

Additionally I can see that waitForReady functionality has already diverged between this fixture and the other one. Why not create a new function waitForRenderReady to avoid this going forward.

Copy link
Copy Markdown
Contributor

@IDCs IDCs left a comment

Choose a reason for hiding this comment

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

Left a couple of comments. Note that I wasn't sure if this PR is ready for review yet as it's in draft, so apologies if I jumped the gun.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

This PR has conflicts. You need to rebase the PR before it can be merged.

@github-actions
Copy link
Copy Markdown

This PR has been marked as stale due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants