Skip to content

Add Selenium tests to Klados#343

Draft
gaurav wants to merge 27 commits into
masterfrom
add-selenium-tests
Draft

Add Selenium tests to Klados#343
gaurav wants to merge 27 commits into
masterfrom
add-selenium-tests

Conversation

@gaurav

@gaurav gaurav commented Jan 15, 2025

Copy link
Copy Markdown
Member

This PR will start the process of adding Selenium tests to Klados, so we can make sure that the core functionality of Klados doesn't break in future changes.

WIP

@gaurav gaurav mentioned this pull request Feb 26, 2025
2 tasks
gaurav and others added 13 commits February 21, 2026 17:21
Replaces the Selenium IDE .side files with Playwright tests that have
better auto-waiting, network mocking, and native dialog support.

- Install @playwright/test; add test/test:ui/test:headed npm scripts
- playwright.config.js: auto-starts dev server, clears cookies per test,
  runs Chromium + Firefox in parallel
- Add data-testid attributes to Sidebar, PhyxView, PhylogenyView,
  PhylorefView for stable selectors
- tests/playwright/basic-demo.spec.js: loads Brochu 2003 example,
  mocks JPhyloRef response, asserts all 6 phylorefs resolve correctly
- tests/playwright/editing.spec.js: creates a Phyx by hand, adds a
  phylogeny with Newick, renames an internal node via window.prompt,
  adds specifiers and verifies sidebar labels
- tests/playwright/fixtures/: shared fixture that mocks JPhyloRef POST
  and aborts Open Tree of Life requests; captured Brochu 2003 response
- tests/playwright/pages/: SidebarPage and PhyxViewPage page objects
- .github/workflows/playwright-tests.yml: CI on push/PR to master

All 6 tests pass on both Chromium and Firefox.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hlapp

hlapp commented Feb 22, 2026

Copy link
Copy Markdown
Member

@gaurav FYI, in other projects we have standardized on AGENTS.md instead of using Copilot or Claude-specific instructions.

@gaurav

gaurav commented Feb 22, 2026

Copy link
Copy Markdown
Member Author

@gaurav FYI, in other projects we have standardized on AGENTS.md instead of using Copilot or Claude-specific instructions.

Thanks for the suggestion! I've incorporated it into #384

Copilot AI left a comment

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.

Pull request overview

This PR introduces end-to-end test coverage for Klados by adding Playwright-based integration tests (plus initial Selenium IDE recordings), and updates the UI with stable data-testid hooks so tests can target elements reliably.

Changes:

  • Add Playwright test suite (page objects, shared fixtures with mocked JPhyloRef, and initial specs for “Basic demo” and editing flows).
  • Add data-testid attributes across key UI components to enable stable E2E selectors (sidebar, phyloref results table, specifier add buttons, phylogeny Newick textarea).
  • Add CI workflow to run Playwright tests, plus Selenium Side Runner scripts/config and Selenium .side recordings.

Reviewed changes

Copilot reviewed 18 out of 20 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/selenium/Klados.side Selenium IDE recording for the “Basic demo” scenario.
tests/selenium/Editing.side Selenium IDE recording for editing scenarios.
tests/playwright/pages/SidebarPage.js Page object encapsulating sidebar actions via data-testid locators.
tests/playwright/pages/PhyxViewPage.js Page object for asserting resolution results in the summary table.
tests/playwright/fixtures/index.js Shared fixture that mocks JPhyloRef responses and blocks OpenTree API calls.
tests/playwright/fixtures/brochu-jphyloref-response.json Mock response data used by Playwright tests.
tests/playwright/basic-demo.spec.js Playwright test translating the Selenium “Basic demo” scenario.
tests/playwright/editing.spec.js Playwright tests for basic phylogeny editing and node rename prompt behavior.
src/components/sidebar/Sidebar.vue Adds data-testid hooks for examples, resolve action, and navigation items.
src/components/phyx/PhyxView.vue Adds per-cell data-testid hooks for phyloref resolution result assertions.
src/components/phyloref/PhylorefView.vue Adds data-testid hooks for “add internal/external specifier” buttons.
src/components/phylogeny/PhylogenyView.vue Adds data-testid for Newick textarea; adjusts default Newick getter behavior.
src/components/specifiers/Specifier.vue Formatting-only change within <option> rendering.
src/components/phylogeny/PhyloTree.vue Refactors context menu item injection to support node renaming via prompt.
public/examples/BasicExample.json Adds a basic example JSON fixture.
playwright.config.js Adds Playwright configuration including webServer startup.
.github/workflows/playwright-tests.yml Adds GitHub Actions workflow to run Playwright tests and upload report artifacts.
package.json Adds Playwright/Selenium dev dependencies and test scripts; bumps phylotree.
.side.yml Adds Selenium Side Runner configuration (Firefox capabilities).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread playwright.config.js
storageState: { cookies: [], origins: [] }, // clear cookies before each test
},
webServer: {
command: 'npm run dev',

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

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

webServer.command runs npm run dev without forcing Vite to stay on port 5173. By default Vite will increment the port if 5173 is in use, but this config hard-codes baseURL/webServer.url to 5173, which can cause Playwright to hang waiting for the wrong port. Consider running Vite with --strictPort --port 5173 (or enabling server.strictPort in Vite config) so the test runner fails fast instead of silently switching ports.

Suggested change
command: 'npm run dev',
command: 'npm run dev -- --strictPort --port 5173',

Copilot uses AI. Check for mistakes.
Comment on lines +14 to +26
const bruchuResponse = JSON.parse(
fs.readFileSync(path.join(__dirname, 'brochu-jphyloref-response.json'), 'utf8')
);

const test = base.extend({
mockedPage: async ({ page }, use) => {
// Mock the JPhyloRef reasoner endpoint.
await page.route('https://reasoner.phyloref.org/reason', async (route) => {
await route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify(bruchuResponse),
});

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

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

Typo in fixture variable name: bruchuResponse appears to refer to the Brochu example/fixture (and the JSON file is brochu-jphyloref-response.json). Renaming to brochuResponse would improve readability and consistency with the test names/comments.

Copilot uses AI. Check for mistakes.
Comment on lines +12 to +14
* 6. Resolve and confirm the phyloreference resolves to Mammalia.
* 7. Add an external specifier (Enteroctopus dofleini) and re-resolve.
* 8. Verify the phyloreference type changed to reflect an external specifier.

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

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

The scenario description in the header comment lists steps 6–8 (resolve, verify resolution, add external specifier/type change), but the test currently stops after asserting the two internal specifiers are visible. Either trim the comment to match what the test actually does, or implement the missing steps so the documentation stays accurate.

Suggested change
* 6. Resolve and confirm the phyloreference resolves to Mammalia.
* 7. Add an external specifier (Enteroctopus dofleini) and re-resolve.
* 8. Verify the phyloreference type changed to reflect an external specifier.

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +27
await page.locator('#curator-name').fill('Gaurav Vaidya');
await page.locator('#curator-email').fill('gaurav@ggvaidya.com');
await page.locator('#external-reference').fill('0000-0003-0587-0454');

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

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

This test hard-codes a real person’s name/email/ORCID in the inputs. Even in test code, it’s better to use clearly fake values (e.g., test@example.com, placeholder ORCID) to avoid inadvertently committing personal data and to make it obvious the values are not meaningful outside the test.

Suggested change
await page.locator('#curator-name').fill('Gaurav Vaidya');
await page.locator('#curator-email').fill('gaurav@ggvaidya.com');
await page.locator('#external-reference').fill('0000-0003-0587-0454');
await page.locator('#curator-name').fill('Test Curator');
await page.locator('#curator-email').fill('test@example.com');
await page.locator('#external-reference').fill('0000-0000-0000-0000');

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +41
"curator": "Gaurav Vaidya",
"curatorEmail": "gaurav@ggvaidya.com",
"curatorORCID": "0000-0003-0587-0454",

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

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

This example file includes a real person’s name/email/ORCID. Consider replacing these with clearly fake/sample values so the repository doesn’t embed personal contact info in fixtures/examples.

Suggested change
"curator": "Gaurav Vaidya",
"curatorEmail": "gaurav@ggvaidya.com",
"curatorORCID": "0000-0003-0587-0454",
"curator": "Example Curator",
"curatorEmail": "curator@example.com",
"curatorORCID": "0000-0000-0000-0000",

Copilot uses AI. Check for mistakes.
Comment on lines +57 to +113
"id": "789a8203-3109-4270-b85d-43f442bb90a8",
"comment": "",
"command": "type",
"target": "id=curator-name",
"targets": [
[
"id=curator-name",
"id"
],
[
"css=#curator-name",
"css:finder"
],
[
"xpath=//input[@id='curator-name']",
"xpath:attributes"
],
[
"xpath=//div[@id='page-content-wrapper']/div/div/div/form/div/div/input",
"xpath:idRelative"
],
[
"xpath=//form/div/div/input",
"xpath:position"
]
],
"value": "Gaurav Vaidya"
},
{
"id": "8098c9dc-dc3f-41a1-89e1-4e2e4384a2da",
"comment": "",
"command": "type",
"target": "id=curator-email",
"targets": [
[
"id=curator-email",
"id"
],
[
"css=#curator-email",
"css:finder"
],
[
"xpath=//input[@id='curator-email']",
"xpath:attributes"
],
[
"xpath=//div[@id='page-content-wrapper']/div/div/div/form/div/div/input[2]",
"xpath:idRelative"
],
[
"xpath=//input[2]",
"xpath:position"
]
],
"value": "gaurav@ggvaidya.com"
},

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

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

This Selenium IDE recording includes real personal data (name/email/ORCID). Please replace these with placeholder values before committing to avoid embedding personal contact info in version control.

Copilot uses AI. Check for mistakes.
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.

3 participants