tests/ui-testing/pages/loginPage.js (2)

1-2: Remove unused import

The expect import from '@playwright/test' is not used in the current implementation. Consider removing it to keep the code clean.

-import { expect } from '@playwright/test';

---

12-14: Add error handling for environment variable

The gotoLoginPage method correctly uses an environment variable for the base URL. However, consider adding error handling in case the environment variable is not set.

 async gotoLoginPage() {
+  if (!process.env["ZO_BASE_URL"]) {
+    throw new Error("ZO_BASE_URL environment variable is not set");
+  }
   await this.page.goto(process.env["ZO_BASE_URL"]);
 }

tests/ui-testing/pages/alertsPage.js (2)

36-40: Remove unnecessary comment

The comment "Set the time filter to the last 30 seconds" doesn't add any value beyond what the method name already implies.

Consider removing this comment:

async setTimeToPast30Seconds() {
-  // Set the time filter to the last 30 seconds
  await this.page.locator(this.dateTimeButton).click();
  await this.relative30SecondsButton.click();
}

---

1-54: Improve code formatting

The file has inconsistent spacing, particularly around class methods and between class members. Consider using a linter or formatter to ensure consistent spacing throughout the file.

Here's an example of how the spacing could be improved:

export class AlertPage {
  constructor(page) {
    this.page = page;
    this.alertMenu = this.page.locator('[data-test="menu-link-\\/alerts-item"]');
    // ... other initializations ...
    this.profileButton = page.locator('button').filter({ hasText: (process.env["ZO_ROOT_USER_EMAIL"]) });
    this.signOutButton = page.getByText('Sign Out');
  }

  async navigateToAlerts() {
    await this.alertMenu.click();
  }

  async createAlert() {
    await this.addAlertButton.click();
    await this.sqlOption.click();
    await this.addTimeRangeButton.click();
  }

  // ... other methods ...
}

tests/ui-testing/pages/tracesPage.js (2)

39-57: Consider removing commented code and improve button selection

The methods for setting and verifying date/time look good overall, but there are a couple of points to consider:

1. There's a commented out wait statement. It's generally better to remove commented code or explain why it might be needed in the future.

2. The use of hard-coded values like '1' for button selection might be fragile. Consider using more specific selectors.

Apply this diff to address these issues:

 async fillTimeRange(startTime, endTime) {
-    await this.page.getByRole('button', { name: '1', exact: true }).click();
+    await this.page.getByRole('button', { name: 'Select start time', exact: true }).click();
     await this.page.getByLabel('access_time').first().fill(startTime);
-    await this.page.getByRole('button', { name: '1', exact: true }).click();
+    await this.page.getByRole('button', { name: 'Select end time', exact: true }).click();
     await this.page.getByLabel('access_time').nth(1).fill(endTime);
-    // await this.page.waitForTimeout(1000);
 }

Replace 'Select start time' and 'Select end time' with the actual text or aria-label of the buttons in your UI.

---

59-66: Sign out method looks good, remove unnecessary empty lines

The sign out method is implemented correctly. However, there are unnecessary empty lines at the end of the file.

Apply this diff to remove the empty lines:

   async signOut() {
     await this.profileButton.click();
     await this.signOutButton.click();
   }
 }
-
-

This will keep the file clean and adhere to common style guidelines.

.github/workflows/playwright.yml (1)

90-90: LGTM! Consider minor formatting improvement.

The addition of "secondsPrecisionAdded.spec.js" to the test files aligns well with the PR objective of updating E2e seconds precision. The multi-line format improves readability.

For consistency, consider adding a space after each comma in the array. This isn't strictly necessary but can improve readability:

testfilename: 
  ["sanity.spec.js", "alerts.spec.js", "schema.spec.js", "logspage.spec.js", "logsqueries.spec.js", "logsquickmode.spec.js", "multiselect-stream.spec.js", "pipeline.spec.js", "dashboardtype.spec.js", "dashboard.spec.js", "visualize.spec.js", "unflattened.spec.js", "secondsPrecisionAdded.spec.js"]

🧰 Tools

🪛 yamllint

[warning] 90-90: too few spaces after comma

(commas)

tests/ui-testing/playwright-tests/secondsPrecisionAdded.spec.js (3)

1-15: Remove commented-out code and unused imports.

The imports and setup look good overall. However, there are some cleanup tasks:

1. Remove the commented-out import on line 13:

-//import {CommomnLocator} from '../pages/CommonLocator'

2. Remove the commented-out console.log on line 15:

-//console.log ('Login Started')

---

17-48: Consider uncommenting the selectIndexAndStream step.

The test structure looks good, but there's a commented-out line that might be important:

// await logsPage.selectIndexAndStream();

If this step is necessary for the test's functionality, consider uncommenting it. If it's not needed, remove the comment entirely.

---

1-447: Overall feedback on test structure and coverage

1. Test Coverage: The file provides comprehensive coverage of various pages (Logs, Traces, Reports, Dashboard, Alerts, Metrics, RUM) and scenarios (relative and absolute time), which is commendable.

2. Consistency: The tests follow a consistent structure, which aids in readability and maintenance. However, this has led to some code duplication that could be refactored (as mentioned in a previous comment).

3. Commented Code: There are several instances of commented-out code and tests. It's recommended to either remove these or uncomment and fix them if they're needed.

4. Error Handling: Consider adding error handling and assertions to ensure the tests fail appropriately when unexpected conditions occur.

5. Waiting Strategy: The tests use page.waitForTimeout() in several places. Consider using more reliable waiting strategies like waitForSelector() or waitForNavigation() where possible.

6. Test Independence: Ensure each test is independent and doesn't rely on the state from previous tests. This may already be the case, but it's worth double-checking.

7. Parameterization: For tests that are very similar (like the relative and absolute time tests for each page), consider using test parameterization to reduce code duplication further.

Overall, this is a solid set of tests that with some refactoring could be even more maintainable and robust.

tests/ui-testing/pages/logsPage.js (3)

19-19: Remove commented-out code to maintain code cleanliness

There are several lines with commented-out code at lines 19, 44, 52, and 91. It's a good practice to remove unused code to keep the codebase clean and maintainable.

Also applies to: 44-44, 52-52, 91-91

---

65-66: Add assertions to confirm actions and improve test reliability

After clicking on elements in lines 65 and 66, consider adding assertions to confirm that the expected changes occurred on the page. For example, verify that the time filter has updated appropriately. This practice enhances test reliability by ensuring that each action produces the intended result.

---

6-30: Constructor could be streamlined for better readability

The constructor defines many locators. To improve readability and maintainability, consider grouping related locators or creating helper methods if appropriate. Additionally, ensure consistent formatting and spacing for better code clarity.

Configuration used: CodeRabbit UI
Review profile: CHILL