Skip to content

Conversation

@neha00290
Copy link
Contributor

@neha00290 neha00290 commented Aug 29, 2025

PR Type

Tests


Description

  • Add timestamp field UI test

  • Extend Logs page selectors and actions

  • Validate schema/infoschema navigation

  • Ensure timestamp visibility after clear


Diagram Walkthrough

flowchart LR
  A["LogsPage selectors/actions"] -- used by --> B["Logs quickmode spec"]
  B -- tests --> C["Click _timestamp, schema, infoschema"]
  C -- asserts --> D["_timestamp visible after clear"]
Loading

File Walkthrough

Relevant files
Tests
logsPage.js
Add timestamp and schema-related UI helpers                           

tests/ui-testing/pages/logsPages/logsPage.js

  • Add selector timestampFieldTable
  • Add actions: click timestamp, schema, infoschema, clear
  • Add assertion: expect timestamp field visible
+21/-0   
logsquickmode.spec.js
Add UI test for _timestamp quick mode flow                             

tests/ui-testing/playwright-tests/Logs/logsquickmode.spec.js

  • Add new test covering _timestamp interaction
  • Navigate schema/infoschema and clear actions
  • Validate kubernetes_pod_id selection and cleanup
  • Assert _timestamp remains visible
+32/-0   

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Selector Robustness

Using getByTitle('_timestamp') assumes a stable title attribute on the field cell; consider a data-test hook to avoid flakiness if titles change or are localized.

async clickTimestampField() {
    return await this.page.locator(this.timestampFieldTable).getByTitle('_timestamp').click();
}

async clickSchemaButton() {
    return await this.page.getByRole('button').filter({ hasText: /^schema$/ }).click();
}

async clickInfoSchemaButton() {
    return await this.page.getByRole('button').filter({ hasText: 'infoschema' }).click();
}

async clickClearButton() {
    return await this.page.getByRole('button', { name: 'Clear' }).click();
}

async expectTimestampFieldVisible() {
    return await expect(this.page.locator(this.timestampFieldTable).getByTitle('_timestamp')).toBeVisible();
}
Test Flakiness

The flow clicks multiple UI controls without explicit waits for view changes (e.g., after switching schema/infoschema, clearing, or adding interesting fields). Add explicit assertions or waitFor* to ensure the UI state has settled before next action.

test("should click timestamp field and then search for kubernetes_pod_id field", {
  tag: ['@fieldInteraction', '@all', '@logs']
}, async ({ page }) => {
  testLogger.info('Testing timestamp field click and kubernetes_pod_id field search');

  // Check if _timestamp field exists and click it
  await pm.logsPage.clickTimestampField();

  // Click on schema button
  await pm.logsPage.clickSchemaButton();

  // Search and click kubernetes_pod_id field
  await pm.logsPage.fillIndexFieldSearchInput("kubernetes_pod_id");
  await pm.logsPage.clickInterestingFieldButton("kubernetes_pod_id");

  // Click on infoschema button
  await pm.logsPage.clickInfoSchemaButton();

  // Click Clear button
  await pm.logsPage.clickClearButton();

  // Remove kubernetes_pod_id as interesting field
  await pm.logsPage.clickInterestingFieldButton("kubernetes_pod_id");

  // Assert that _timestamp still exists
  await pm.logsPage.expectTimestampFieldVisible();

  testLogger.info('Timestamp field click and kubernetes_pod_id field search completed');
});
Case-Sensitive Match

clickSchemaButton uses a regex /^schema$/ while clickInfoSchemaButton uses a plain 'infoschema' text filter; ensure consistency and avoid case/localization issues by targeting role+name with exact: true or data-test selectors.

async clickSchemaButton() {
    return await this.page.getByRole('button').filter({ hasText: /^schema$/ }).click();
}

async clickInfoSchemaButton() {
    return await this.page.getByRole('button').filter({ hasText: 'infoschema' }).click();
}

@github-actions
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Ensure visibility before clicking

Add an assertion to ensure the _timestamp field is visible/enabled before clicking
to avoid flaky test failures. Also wait for the click to complete by asserting a
post-click state if applicable.

tests/ui-testing/pages/logsPages/logsPage.js [1923-1925]

 async clickTimestampField() {
-    return await this.page.locator(this.timestampFieldTable).getByTitle('_timestamp').click();
+    const ts = this.page.locator(this.timestampFieldTable).getByTitle('_timestamp');
+    await expect(ts).toBeVisible();
+    await ts.click();
 }
Suggestion importance[1-10]: 7

__

Why: Adding a visibility assertion before clicking the _timestamp field can reduce flakiness and aligns with Playwright best practices; the improved code accurately adapts the existing snippet.

Medium
General
Harden schema button selector

Use a case-insensitive, exact match to prevent accidental matches and flakiness due
to capitalization or surrounding text. Also ensure visibility before clicking.

tests/ui-testing/pages/logsPages/logsPage.js [1927-1929]

 async clickSchemaButton() {
-    return await this.page.getByRole('button').filter({ hasText: /^schema$/ }).click();
+    const btn = this.page.getByRole('button', { name: /^(?i)schema$/ });
+    await expect(btn).toBeVisible();
+    await btn.click();
 }
Suggestion importance[1-10]: 3

__

Why: Improving robustness by asserting visibility is reasonable, but the proposed case-insensitive regex (?i) is not valid in JavaScript, making the locator incorrect; only part of the suggestion is useful.

Low
Make infoschema locator robust

Narrow the locator with an accessible name regex and wait for visibility to avoid
clicking unintended elements or hidden buttons. This reduces test flakiness.

tests/ui-testing/pages/logsPages/logsPage.js [1931-1933]

 async clickInfoSchemaButton() {
-    return await this.page.getByRole('button').filter({ hasText: 'infoschema' }).click();
+    const btn = this.page.getByRole('button', { name: /^(?i)infoschema$/ });
+    await expect(btn).toBeVisible();
+    await btn.click();
 }
Suggestion importance[1-10]: 3

__

Why: Visibility assertion is helpful, but the (?i) inline case-insensitive flag is invalid in JS regex, so the suggested locator would fail; partial benefit only.

Low

@neha00290 neha00290 marked this pull request as ready for review August 29, 2025 11:27
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR adds comprehensive UI testing functionality for timestamp field interactions in the OpenObserve logs interface. The changes extend the existing test automation framework with two key components:

Test Infrastructure Enhancement (logsPage.js):
The LogsPage class gains new testing capabilities through the addition of a timestampFieldTable selector that targets the log search index list fields table. Five new methods are introduced following the established Page Object Model pattern:

  • clickTimestampField() - Interacts with the _timestamp field in the table
  • clickSchemaButton() and clickInfoSchemaButton() - Navigate between schema views
  • clickClearButton() - Triggers clearing operations
  • expectTimestampFieldVisible() - Provides assertion capabilities for timestamp field visibility

Test Implementation (logsquickmode.spec.js):
A new test case "should be able to click on _timestamp qm" validates the complete workflow of timestamp field interaction in quick mode. The test orchestrates a sequence of UI actions: clicking the timestamp field, navigating through schema and infoschema views, performing clear operations, and ultimately asserting that the timestamp field remains visible throughout these operations.

This enhancement fits into OpenObserve's broader UI testing strategy by providing reusable, maintainable test components that can validate critical timestamp functionality across different user workflows. The implementation follows existing patterns in the codebase, using Playwright's locator and action APIs consistently with other test utilities.

Confidence score: 5/5

  • This PR is extremely safe to merge with minimal risk as it only adds test code without touching production functionality
  • Score reflects the addition of well-structured test code that follows established patterns and doesn't modify any production logic
  • No files require special attention as the changes are isolated to test infrastructure and follow existing conventions

1 file reviewed, no comments

Edit Code Review Bot Settings | Greptile

@neha00290 neha00290 merged commit b04099a into main Aug 29, 2025
28 checks passed
@neha00290 neha00290 deleted the e2e-qm-timestamp branch August 29, 2025 12:14
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.

4 participants