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

9-10: LGTM! Consider documenting the locator strategy change.

The switch to data-cy attribute is a good practice for test stability. Consider adding a brief comment explaining why this selector strategy was chosen over the role-based approach.

-    //this.loginButton = this.page.getByRole('button', { name: 'Login' });
+    // Using data-cy attribute for better test stability and maintainability
     this.loginButton = page.locator('[data-cy="login-sign-in"]');

tests/ui-testing/playwright-tests/reportsUpdated.spec.js (1)

8-10: Consider removing "Updated" suffix from test suite name.

The suffix doesn't add value and might cause confusion. Consider renaming to just "Report test cases".

-test.describe("Report test cases Updated", () => {
+test.describe("Report test cases", () => {

tests/ui-testing/pages/ingestionPage.js (1)

34-34: Use optional chaining for null safety.

The condition can be improved using optional chaining to handle potential null values.

- if (contentType && contentType.includes("application/json")) {
+ if (contentType?.includes("application/json")) {

🧰 Tools

🪛 Biome

[error] 34-34: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

.github/workflows/playwright.yml (2)

90-94: Consider improving the test file matrix configuration.

The test file list is now split across multiple lines, which improves readability. However, to make it more maintainable:

1. Consider using a consistent indentation level
2. Add commas consistently after each entry
3. Consider grouping related test files together (e.g., logs-related, dashboard-related)

Apply this diff to improve the configuration:

         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","reportsUpdated.spec.js"]
+          [
+            # Core functionality
+            "sanity.spec.js",
+            "alerts.spec.js",
+            "schema.spec.js",
+            # Logs functionality
+            "logspage.spec.js",
+            "logsqueries.spec.js",
+            "logsquickmode.spec.js",
+            "multiselect-stream.spec.js",
+            # Dashboard & Visualization
+            "dashboardtype.spec.js",
+            "dashboard.spec.js",
+            "visualize.spec.js",
+            # Other features
+            "pipeline.spec.js",
+            "unflattened.spec.js",
+            "secondsPrecisionAdded.spec.js",
+            "reportsUpdated.spec.js"
+          ]

🧰 Tools

🪛 yamllint

[error] 90-90: trailing spaces

(trailing-spaces)

---

[warning] 91-91: wrong indentation: expected 11 but found 10

(indentation)

---

[error] 92-92: trailing spaces

(trailing-spaces)

---

[warning] 93-93: wrong indentation: expected 11 but found 10

(indentation)

---

[warning] 94-94: wrong indentation: expected 11 but found 10

(indentation)

---

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

(commas)

---

90-94: Fix formatting issues in the test file matrix.

The static analysis tools have identified several formatting issues:

1. Remove trailing spaces
2. Fix indentation to be consistent (11 spaces)
3. Add space after comma in the last entry

Apply this diff to fix the formatting:

         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","reportsUpdated.spec.js"]
+          ["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", "reportsUpdated.spec.js"]

🧰 Tools

🪛 yamllint

[error] 90-90: trailing spaces

(trailing-spaces)

---

[warning] 91-91: wrong indentation: expected 11 but found 10

(indentation)

---

[error] 92-92: trailing spaces

(trailing-spaces)

---

[warning] 93-93: wrong indentation: expected 11 but found 10

(indentation)

---

[warning] 94-94: wrong indentation: expected 11 but found 10

(indentation)

---

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

(commas)

tests/ui-testing/pages/dashboardPage.js (5)

10-10: Enhance dashboard name readability.

Currently, dashboardName is set to dashboard${Date.now()}, which results in a numerical suffix that's not easily readable. Consider formatting the date to improve clarity, such as using a timestamp or a formatted date string.

Apply this diff to enhance readability:

- this.dashboardName = `dashboard${Date.now()}`;
+ const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
+ this.dashboardName = `dashboard-${timestamp}`;

---

60-60: Handle potential changes in data-test IDs.

The data-test ID 'field-list-item-logs-default-k8s_app_instance' is quite specific and may change if underlying data changes. Consider parameterizing or making the selector more robust.

Apply this diff to generalize the selector:

- await this.page.locator('[data-test="field-list-item-logs-default-k8s_app_instance"] [data-test="dashboard-add-y-data"]').click();
+ await this.page.locator('[data-test^="field-list-item-logs-default"] [data-test="dashboard-add-y-data"]').click();

---

53-59: Use consistent and descriptive panel names.

The panel name 'AutoP' might not be descriptive. For better maintainability and readability, use a name that reflects the panel's purpose.

Consider updating the panel name:

- await this.page.locator('[data-test="dashboard-panel-name"]').fill('AutoP');
+ await this.page.locator('[data-test="dashboard-panel-name"]').fill('AutoGeneratedPanel');

---

34-65: Factor out repeated locators and actions into helper methods.

The createDashboard() method contains repetitive code for interacting with page elements. Refactoring these into helper methods can improve code reusability and readability.

---

10-16: Group locators logically and consider page object best practices.

Organize locators related to similar functionalities together. Additionally, consider using getter methods for locators to ensure they are instantiated fresh each time, which can prevent stale element issues.

Example:

- this.dashboardNameInput = page.locator('[data-test="add-dashboard-name"]');
+ get dashboardNameInput() {
+   return this.page.locator('[data-test="add-dashboard-name"]');
+ }

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

55-55: Avoid using fixed timeouts with 'waitForTimeout'

Using waitForTimeout with fixed delays can lead to flaky tests and longer execution times. It's better to wait for specific events or elements to ensure synchronization.

Replace waitForTimeout with more reliable synchronization methods. For example:

- await this.page.waitForTimeout(5000);
+ await this.page.waitForLoadState('networkidle');

Ensure that you're waiting for the necessary conditions before proceeding.

Also applies to: 63-63, 69-69

---

54-54: Parameterize hardcoded values in input fields

Hardcoding values like "rreport1" and "reporterTest" reduces code flexibility. Consider passing these values as parameters or using constants.

Refactor the code to accept reportName and reportTitle as parameters:

- await this.reportNameInput.fill("rreport1");
+ await this.reportNameInput.fill(reportName);

- await this.titleInput.fill("reporterTest");
+ await this.titleInput.fill(reportTitle);

Update the method signature accordingly.

Also applies to: 76-76

---

43-43: Remove commented-out code for clarity

The line //await this.reportsMenu.click(); is commented out. If it's no longer needed, consider removing it to keep the code clean and maintainable.

- //await this.reportsMenu.click();

If the line is needed for future functionality, add a comment explaining why it's commented out.

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Description: Verify that ZO_ROOT_USER_EMAIL is defined.

# Test: Check for environment variable. Expect: Variable is set.
if [[ -z "${ZO_ROOT_USER_EMAIL}" ]]; then
  echo "Environment variable ZO_ROOT_USER_EMAIL is not set."
  exit 1
else
  echo "ZO_ROOT_USER_EMAIL is set to ${ZO_ROOT_USER_EMAIL}."
fi

- this.successAlert = page.getByRole('alert').nth(1);
+ this.successAlert = page.getByRole('alert').nth(0);

`[data-test="report-list-${reportName}-delete-report"]`

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

57-74: Simplify stream selection logic

The current implementation uses complex selector chains. Consider using data-test attributes for stream selection elements.

// Add these data-test attributes to the UI elements
const streamDropdown = page.locator('[data-test="stream-dropdown"]');
const streamOption = page.locator('[data-test="stream-option-e2e_automate"]');

---

87-91: Remove commented code

Remove commented-out code to improve readability. If these lines represent alternative approaches or TODOs, they should be documented in issues/tickets instead.

Also applies to: 96-97, 102-103

Configuration used: CodeRabbit UI
Review profile: CHILL

#!/bin/bash
# Search for organization-related configuration or setup
rg -g '!node_modules' -g '*.{js,json,yaml,yml}' -i 'default.*organization'

# Look for other test files that might be setting up organizations
fd -e js -e ts | xargs rg 'organization.*setup|setup.*organization'

#!/bin/bash
# Let's check for organization-related test setup and configuration
rg -g '!node_modules' "beforeAll|beforeEach" tests/ui-testing/playwright-tests/ -B 2 -A 10

# Look specifically at the login flow in test files
rg -g '!node_modules' "login.*page|loginPage" tests/ui-testing/playwright-tests/ -B 2 -A 10

# Check for organization selection in other test files
rg -g '!node_modules' "organizations-select|getByRole.*organization" tests/ui-testing/

#!/bin/bash
# Let's check how the organization is set in the environment and test setup
rg -g '!node_modules' "ORGNAME|org_identifier" tests/ui-testing/

# Check for organization selection implementation in the login page
cat tests/ui-testing/pages/loginPage.js

# Look for organization-related environment setup
fd -e json -e js -e yml -e yaml | xargs rg "organization.*default|default.*organization"

#!/bin/bash
# Check the structure of test data
cat tests/test-data/logs_data.json | jq 'type'

# Look for similar ingestion tests to compare data structure
rg -l 'streamName.*_json' tests/

#!/bin/bash
# Check the content structure of logs_data.json
cat tests/test-data/logs_data.json | jq '.[0]'

# Look for usage of this data in tests
rg -A 5 "logs_data.json" tests/ui-testing/