Skip to content

Commit 3d7cb2b

Browse files
committed
refactor: improve test reliability and simplify filter tests
- 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.
1 parent d0ac38d commit 3d7cb2b

File tree

4 files changed

+59
-55
lines changed

4 files changed

+59
-55
lines changed

packages/app/tests/e2e/components/SavedSearchModalComponent.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Used for creating and managing saved searches
44
* Not used until Saved Search functionality is implemented
55
*/
6-
import { Locator, Page } from '@playwright/test';
6+
import { expect, Locator, Page } from '@playwright/test';
77

88
export class SavedSearchModalComponent {
99
readonly page: Page;
@@ -69,6 +69,34 @@ export class SavedSearchModalComponent {
6969
await this.submit();
7070
}
7171

72+
/**
73+
* Save search and wait for URL to change to the saved search page
74+
* This is more reliable than waiting separately for modal close and URL change
75+
*/
76+
async saveSearchAndWaitForNavigation(
77+
name: string,
78+
tags: string[] = [],
79+
): Promise<void> {
80+
await this.fillName(name);
81+
82+
for (const tag of tags) {
83+
await this.addTag(tag);
84+
}
85+
86+
// Start waiting for URL change BEFORE clicking submit to avoid race condition
87+
const urlPromise = this.page.waitForURL(/\/search\/[a-f0-9]+/, {
88+
timeout: 15000,
89+
});
90+
91+
await this.submit();
92+
93+
// Wait for navigation to complete
94+
await urlPromise;
95+
96+
// Optionally wait for modal to fully close
97+
await expect(this.container).toBeHidden();
98+
}
99+
72100
/**
73101
* Get tag elements
74102
*/

packages/app/tests/e2e/core/navigation.spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ test.describe('Navigation', { tag: ['@core'] }, () => {
7171
await test.step('Navigate between each page', async () => {
7272
for (const { testId, contentTestId } of navLinks) {
7373
const link = page.locator(`[data-testid="${testId}"]`);
74+
await link.scrollIntoViewIfNeeded();
7475
await link.click();
7576

7677
const content = page.locator(`[data-testid="${contentTestId}"]`);

packages/app/tests/e2e/features/search/saved-search.spec.ts

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
import {
2+
DEFAULT_LOGS_SOURCE_NAME,
3+
DEFAULT_TRACES_SOURCE_NAME,
4+
} from 'tests/e2e/utils/constants';
5+
16
import { SearchPage } from '../../page-objects/SearchPage';
27
import { expect, test } from '../../utils/base-test';
38

@@ -34,10 +39,9 @@ test.describe('Saved Search Functionality', { tag: '@full-stack' }, () => {
3439
await searchPage.setCustomSELECT(customSelect);
3540
await searchPage.submitEmptySearch();
3641
await searchPage.openSaveSearchModal();
37-
await searchPage.savedSearchModal.saveSearch('Custom Select Search');
38-
39-
await expect(searchPage.savedSearchModal.container).toBeHidden();
40-
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 15000 });
42+
await searchPage.savedSearchModal.saveSearchAndWaitForNavigation(
43+
'Custom Select Search',
44+
);
4145
});
4246

4347
const savedSearchAUrl = page.url().split('?')[0];
@@ -48,10 +52,9 @@ test.describe('Saved Search Functionality', { tag: '@full-stack' }, () => {
4852
// Keep default SELECT (don't modify it)
4953
await searchPage.submitEmptySearch();
5054
await searchPage.openSaveSearchModal();
51-
await searchPage.savedSearchModal.saveSearch('Default Select Search');
52-
53-
await expect(searchPage.savedSearchModal.container).toBeHidden();
54-
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 10000 });
55+
await searchPage.savedSearchModal.saveSearchAndWaitForNavigation(
56+
'Default Select Search',
57+
);
5558
});
5659

5760
await test.step('Navigate back to first saved search', async () => {
@@ -87,12 +90,9 @@ test.describe('Saved Search Functionality', { tag: '@full-stack' }, () => {
8790
await searchPage.setCustomSELECT(customSelect);
8891
await searchPage.submitEmptySearch();
8992
await searchPage.openSaveSearchModal();
90-
await searchPage.savedSearchModal.saveSearch(
93+
await searchPage.savedSearchModal.saveSearchAndWaitForNavigation(
9194
'Custom Select Source Test',
9295
);
93-
94-
await expect(searchPage.savedSearchModal.container).toBeHidden();
95-
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 5000 });
9696
});
9797

9898
const savedSearchUrl = page.url().split('?')[0];
@@ -132,25 +132,20 @@ test.describe('Saved Search Functionality', { tag: '@full-stack' }, () => {
132132
'should use default SELECT when switching sources within a saved search',
133133
{ tag: '@full-stack' },
134134
async ({ page }) => {
135-
let originalSourceName: string | null = null;
136-
137135
await test.step('Create and navigate to saved search', async () => {
138136
const customSelect =
139137
'Timestamp, Body, lower(ServiceName) as service_name';
140138
await searchPage.setCustomSELECT(customSelect);
141139
await searchPage.submitEmptySearch();
142140
await searchPage.openSaveSearchModal();
143-
await searchPage.savedSearchModal.saveSearch('Source Switching Test');
144-
145-
await expect(searchPage.savedSearchModal.container).toBeHidden();
146-
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 5000 });
141+
await searchPage.savedSearchModal.saveSearchAndWaitForNavigation(
142+
'Source Switching Test',
143+
);
147144
});
148145

149146
await test.step('Switch to different source via dropdown', async () => {
150-
originalSourceName = await searchPage.currentSource.inputValue();
151-
152147
await searchPage.sourceDropdown.click();
153-
await searchPage.otherSources.first().click();
148+
await searchPage.selectSource(DEFAULT_TRACES_SOURCE_NAME);
154149
await page.waitForLoadState('networkidle');
155150
await searchPage.table.waitForRowsToPopulate();
156151
});
@@ -167,7 +162,7 @@ test.describe('Saved Search Functionality', { tag: '@full-stack' }, () => {
167162

168163
await test.step('Switch back to original source via dropdown', async () => {
169164
await searchPage.sourceDropdown.click();
170-
await searchPage.selectSource(originalSourceName!);
165+
await searchPage.selectSource(DEFAULT_LOGS_SOURCE_NAME);
171166
await page.waitForLoadState('networkidle');
172167
await searchPage.table.waitForRowsToPopulate();
173168
});
@@ -280,13 +275,10 @@ test.describe('Saved Search Functionality', { tag: '@full-stack' }, () => {
280275
await searchPage.setCustomSELECT(customSelect);
281276
await searchPage.performSearch('ServiceName:frontend');
282277
await searchPage.openSaveSearchModal();
283-
await searchPage.savedSearchModal.saveSearch(
278+
await searchPage.savedSearchModal.saveSearchAndWaitForNavigation(
284279
'Custom Select Navigation Test',
285280
);
286281

287-
await expect(searchPage.savedSearchModal.container).toBeHidden();
288-
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 5000 });
289-
290282
savedSearchUrl = page.url().split('?')[0];
291283
});
292284

@@ -326,10 +318,9 @@ test.describe('Saved Search Functionality', { tag: '@full-stack' }, () => {
326318
await test.step('Create and save a search', async () => {
327319
await searchPage.performSearch('SeverityText:info');
328320
await searchPage.openSaveSearchModal();
329-
await searchPage.savedSearchModal.saveSearch('Browser Navigation Test');
330-
331-
await expect(searchPage.savedSearchModal.container).toBeHidden();
332-
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 5000 });
321+
await searchPage.savedSearchModal.saveSearchAndWaitForNavigation(
322+
'Browser Navigation Test',
323+
);
333324
});
334325

335326
await test.step('Navigate to sessions page', async () => {
@@ -383,12 +374,9 @@ test.describe('Saved Search Functionality', { tag: '@full-stack' }, () => {
383374
// Submit and save the search
384375
await searchPage.submitEmptySearch();
385376
await searchPage.openSaveSearchModal();
386-
await searchPage.savedSearchModal.saveSearch(
377+
await searchPage.savedSearchModal.saveSearchAndWaitForNavigation(
387378
'ORDER BY Multiple Source Switch Test',
388379
);
389-
390-
await expect(searchPage.savedSearchModal.container).toBeHidden();
391-
await page.waitForURL(/\/search\/[a-f0-9]+/, { timeout: 5000 });
392380
});
393381

394382
await test.step('Switch to second source', async () => {

packages/app/tests/e2e/features/search/search-filters.spec.ts

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -47,26 +47,13 @@ test.describe('Search Filters', { tag: ['@search'] }, () => {
4747
});
4848

4949
test('Should search for and apply filters', async () => {
50-
// Use filter component's helper to find a filter with search capability
51-
const skipFilters = ['severity', 'level'];
52-
const filterName =
53-
await searchPage.filters.findFilterWithSearch(skipFilters);
54-
55-
if (filterName) {
56-
// Search input is already visible from findFilterWithSearch
57-
// Test the search functionality
58-
await searchPage.filters.searchFilterValues(filterName, 'test');
59-
60-
// Verify search input has the value
61-
const searchInput = searchPage.filters.getFilterSearchInput(filterName);
62-
await expect(searchInput).toHaveValue('test');
63-
64-
// Clear the search
65-
await searchPage.filters.clearFilterSearch(filterName);
66-
67-
// Verify search input is cleared
68-
await expect(searchInput).toHaveValue('');
69-
}
50+
const filterName = 'SeverityText';
51+
await searchPage.filters.openFilterGroup(filterName);
52+
await searchPage.filters.searchFilterValues(filterName, 'test');
53+
const searchInput = searchPage.filters.getFilterSearchInput(filterName);
54+
await expect(searchInput).toHaveValue('test');
55+
await searchPage.filters.clearFilterSearch(filterName);
56+
await expect(searchInput).toHaveValue('');
7057
});
7158

7259
test('Should pin filter and verify it persists after reload', async () => {

0 commit comments

Comments
 (0)