Skip to content

Conversation

@teeohhem
Copy link
Contributor

@teeohhem teeohhem commented Feb 6, 2026

TLDR: This PR changes playwright full-stack tests to run against a local clickhouse instance (with seeded data) instead of relying on the clickhouse demo server, which can be unpredictable at times. This workflow allows us to fully control the data to make tests more predictable.

This PR:

  • Adds local CH instance to the e2e dockerfile
  • Adds a schema creation script
  • Adds a data seeding script
  • Updates playwright config
  • Updates various tests to change hardcoded fields, metrics, or areas relying on play demo data
  • Updates github workflow to use the dockerfile instead of separate services
  • Runs against a local clickhouse instead of the demo server

Fixes: HDX-3193

@changeset-bot
Copy link

changeset-bot bot commented Feb 6, 2026

⚠️ No Changeset found

Latest commit: f0a2f64

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Feb 6, 2026

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

Project Deployment Actions Updated (UTC)
hyperdx-v2-oss-app Ready Ready Preview, Comment Feb 13, 2026 2:31pm

Request Review

@teeohhem teeohhem changed the title Tom/e2e local ch minimal feat: Use local clickhouse instance for playwright tests Feb 6, 2026
@teeohhem teeohhem changed the title feat: Use local clickhouse instance for playwright tests chore: Use local clickhouse instance for playwright tests Feb 6, 2026
@claude
Copy link

claude bot commented Feb 6, 2026

Code Review

No critical issues found.

Minor observations:

  • seed-clickhouse.ts:24-29: ClickHouse credentials passed via URL params → Consider using HTTP headers for better security (though acceptable for local E2E tests)
  • seed-clickhouse.ts:154,274: Only first 10 logs/sessions linked to traces → This is intentional for testing trace correlation
  • docker-compose.yml:24: CLICKHOUSE_DEFAULT_ACCESS_MANAGEMENT=1 enables dynamic user management → Appropriate for E2E isolated environment
  • init-db-e2e.sh:56-57: 30-day TTL on all test tables → Good practice to prevent data accumulation

Strengths:

  • Proper separation of E2E tables (e2e_ prefix) to avoid collision with dev data
  • Comprehensive test data generation (logs, traces, sessions, K8s metrics)
  • Good error handling and timeout configuration throughout
  • Seeding uses time window that includes future data to prevent flaky tests
  • Fixtures centralized in e2e-fixtures.json for consistency

The PR successfully replaces dependency on external demo server with local ClickHouse, improving test reliability and control.

- Add Docker Compose service for local ClickHouse instance
- Add init script to create e2e_* tables in ClickHouse
- Add seed-clickhouse.ts to populate test data
- Add global-setup-local.ts for local mode setup
- Update test scripts to support local ClickHouse
- Update .env.e2e to point to local ClickHouse instead of demo server
- Add stop-e2e.sh script for cleanup

This enables E2E tests to run against a local Docker ClickHouse instance
with seeded test data, instead of relying on the public demo server.
Both full-stack mode (MongoDB + API + ClickHouse) and local mode
(frontend + ClickHouse) now use the same local ClickHouse instance.
The global-setup-fullstack.ts was missing the seedClickHouse() call,
which meant tests had no data to work with.
- Add saveSearchAndWaitForNavigation() to avoid race condition
  - Starts waiting for URL change BEFORE submitting form
  - Eliminates flaky tests where modal closes but URL hasn't changed yet
  - Replaces manual pattern of await modal hidden + await URL change

- Simplify search-filters.spec.ts to use known test data
  - Use hardcoded 'info' severity from seeded data instead of dynamic discovery
  - Remove 18 lines of complex conditional logic
  - More reliable and easier to understand

These changes leverage consistent test data from ClickHouse seeding.
@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2026

E2E Test Results

1 test failed • 65 passed • 4 skipped • 791s

Status Count
✅ Passed 65
❌ Failed 1
⚠️ Flaky 0
⏭️ Skipped 4

Tests ran across 4 shards in parallel.

View full report →

await test.step('Navigate between each page', async () => {
for (const { testId, contentTestId } of navLinks) {
const link = page.locator(`[data-testid="${testId}"]`);
await link.scrollIntoViewIfNeeded();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reduces flakiness

env.DEFAULT_CONNECTIONS = JSON.stringify(fixture.connections ?? []);
env.DEFAULT_SOURCES = JSON.stringify(fixture.sources ?? []);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

simplify loading connections/sources instead of one big blob in an env file

// Optionally wait for modal to fully close
await expect(this.container).toBeHidden();
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

abstraction for saved searches (lots of flakiness exposed when moving away from play demo server)


await expect(searchPage.savedSearchModal.container).toBeHidden();
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 5000 });
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

simplify these tests with more predictable input

@teeohhem teeohhem marked this pull request as ready for review February 11, 2026 00:52
@teeohhem teeohhem requested a review from pulpdrew February 11, 2026 00:52
run:
docker compose -p e2e -f packages/app/tests/e2e/docker-compose.yml
down -v

Copy link
Contributor Author

Choose a reason for hiding this comment

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

use docker file instead

ORDER BY (ServiceName, MetricName, Attributes, toUnixTimestamp64Nano(TimeUnix))
TTL toDate(TimeUnix) + toIntervalDay(30)
SETTINGS index_granularity = 8192, ttl_only_drop_parts = 1
EOFSQL
Copy link
Contributor Author

Choose a reason for hiding this comment

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

create schema for local db


# DEFAULT_CONNECTIONS and DEFAULT_SOURCES are injected from packages/app/tests/e2e/fixtures/e2e-fixtures.json
# by the e2e API runner (run-api-with-fixtures.js) and by base-test for local mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved to fixtures

//verify there is a pin icon
const pinIcon = searchPage.page.getByTestId(
`filter-pin-${availableFilterValue!}-pinned`,
`filter-pin-${TEST_FILTER_VALUE}-pinned`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

greatly simplify this logic now that we completely control the data

@@ -1,3 +1,5 @@
// We may want to add logs/metrics source names here in the future.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
// We may want to add logs/metrics source names here in the future.

@@ -0,0 +1,10 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this file used anywhere?

@@ -0,0 +1,209 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

I had to run chmod +x docker/clickhouse/local/init-db-e2e.sh to get this script to run in docker desktop - are you able to run the same and commit the extra permission to this PR?

Makefile Outdated
if [ -n "$(tags)" ]; then set -- "$$@" --tags "$(tags)"; fi; \
if [ "$(ui)" = "true" ]; then set -- "$$@" --ui; fi; \
./scripts/test-e2e.sh "$$@"
@# For more control (--ui, --last-failed, --headed, etc), call the script directly:
Copy link
Contributor

Choose a reason for hiding this comment

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

packages/app/tests/e2e/README.md still suggests make e2e ui=true, could that be updated?

const rows: string[] = [];

for (let i = 0; i < count; i++) {
const timestampNs = (now - i * 60000) * 1000000; // 1 minute intervals
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing I noticed while running the tests locally for a few minutes is that tests (the search tests for example) quickly start failing because data is not continuously being inserted for newer/more current timestamps, so when a search filters for "last 5 minutes" there is no data, if seeding ran >5 minutes ago. Would it make sense to see data "in the future" as well, to prevent this sort of thing in most cases?

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.

2 participants