Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(agora): migrate Agora e2e tests (AG-1609) #2965

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

hallieswan
Copy link
Collaborator

@hallieswan hallieswan commented Jan 14, 2025

Description

Migrates Agora end-to-end tests to the monorepo.

Related Issue

Changelog

  • Adds Playwright to agora-app:
    nx g @nx/playwright:configuration --project=agora-app --webServerCommand="nx run agora-apex:serve-detach" --webServerAddress="http://localhost:8000" --linter="eslint"
    
  • Updates to latest Playwright version: pnpm update @playwright/test --latest
  • Installs Playwright browsers and system dependencies as part of workspace-install
  • Migrates existing Agora e2e tests to monorepo, but skips GCT tests until AG-1618 is fixed
  • Lints agora-app e2e tests per Playwright best practices

Preview

Run e2e tests when agora containers are not yet running -- Playwright will start the services before running the tests:

$ nx run agora-app:e2e

> nx run agora-app:e2e


 NX   Ensuring Playwright is installed.

use --skipInstall to skip installation.
[WebServer]  Container agora-api-docs  Creating
[WebServer]  Container agora-mongo  Creating
[WebServer]  Container agora-api-docs  Created
[WebServer]  Container agora-mongo  Created
[WebServer]  Container agora-data  Creating
[WebServer]  Container agora-api  Creating


[WebServer]  Container agora-app  Creating
[WebServer]  Container agora-app  Created

[WebServer]  Container agora-apex  Created
[WebServer]  Container agora-api-docs  Starting
[WebServer]  Container agora-mongo  Starting
[WebServer]  Container agora-api-docs  Started
[WebServer]  Container agora-mongo  Started
[WebServer]  Container agora-api  Starting
[WebServer]  Container agora-data  Starting
[WebServer]  Container agora-data  Started
[WebServer]  Container agora-api  Started
[WebServer]  Container agora-data  Waiting
[WebServer]  Container agora-api  Waiting
[WebServer]  Container agora-api  Healthy
[WebServer]  Container agora-data  Exited
[WebServer]  Container agora-app  Starting
[WebServer]  Container agora-app  Started

[WebServer]  Container agora-app  Waiting
[WebServer]  Container agora-api-docs  Waiting
[WebServer]  Container agora-api-docs  Healthy

[WebServer]  Container agora-app  Healthy
[WebServer]  Container agora-apex  Starting
[WebServer]  Container agora-apex  Started


Running 14 tests using 4 workers
  14 passed (1.7m)

To open last HTML report run:

  npx playwright show-report


——————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 NX   Successfully ran target e2e for project agora-app (2m)

Run e2e tests when agora containers are already running -- Playwright will reuse the existing services:

$ nx run agora-app:e2e

> nx run agora-app:e2e


 NX   Ensuring Playwright is installed.

use --skipInstall to skip installation.


Running 14 tests using 4 workers
  14 passed (25.6s)

To open last HTML report run:

  npx playwright show-report


—————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 NX   Successfully ran target e2e for project agora-app (30s)

Current Playwright report:

$ npx playwright show-report

playwright_report

… starts and waits for agora-apex before running tests
"project": "./apps/agora/app/tsconfig.*?.json"
},
"rules": {
"@typescript-eslint/no-floating-promises": "error",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Recommended by Playwright's best practices

},
"rules": {
"@typescript-eslint/no-floating-promises": "error",
"playwright/no-nested-step": "off",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I often use nested steps to better organize complicated test, e.g. step to wait for GCT page to load, step to close GCT help dialog, step to change GCT dropdown, etc... it's helpful for identifying failures in long running tests with many actions. If we land on a different convention in the future, we can turn on this rule!

"rules": {
"@typescript-eslint/no-floating-promises": "error",
"playwright/no-nested-step": "off",
"playwright/no-conditional-in-test": "off",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've found conditionals to be useful to handle temporary components, such as auto-closing toasts, where the environment can impact whether the component is still visible when the test reaches the step to dismiss it.

"@typescript-eslint/no-floating-promises": "error",
"playwright/no-nested-step": "off",
"playwright/no-conditional-in-test": "off",
"playwright/expect-expect": "off"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This rule enforces that every test has at least one expect call. Many of our tests use functions to encapsulate expectations rather than calling expect directly. We should be able to specify these functions in the rule configuration, but I was unable to get this working, so turning off for now.

Comment on lines +20 to +37
test('has title', async ({ page }) => {
await page.goto('/genes/ENSG00000178209/similar');

// Expect a title "to contain" a substring.
await expect(page).toHaveTitle('Agora');
});

test('has true and false values for nominated target column', async ({ page }) => {
await page.goto('/genes/ENSG00000178209/similar');

await expect(page.locator('table')).toBeVisible();

// sort forward on nominated target
await page.getByRole('cell', { name: 'Nominated Target' }).click();

const cell = await page.getByRole('row').nth(1).getByRole('cell').nth(1).innerText();
expect(cell).not.toBe('');
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved from similar-genes.spec.ts to colocate all similar genes tests within one file.

@@ -0,0 +1,30 @@
import { expect, test } from '@playwright/test';

test('can search for gene', async ({ page }) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added as a proof of concept e2e test in Agora.

@hallieswan hallieswan marked this pull request as ready for review January 14, 2025 23:15
@hallieswan hallieswan requested review from a team and tschaffter as code owners January 14, 2025 23:15
@hallieswan hallieswan self-assigned this Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant