Skip to content

Unskip the bulk import playwright test#30458

Open
Rohit0301 wants to merge 2 commits into
mainfrom
unskip-bulk-import-test
Open

Unskip the bulk import playwright test#30458
Rohit0301 wants to merge 2 commits into
mainfrom
unskip-bulk-import-test

Conversation

@Rohit0301

@Rohit0301 Rohit0301 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Describe your changes:

Fixes #

I worked on ... because ...

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

High-level design:

N/A — small change.

Tests:

Use cases covered

Unit tests

Backend integration tests

Ingestion integration tests

Playwright (UI) tests

Manual testing performed

UI screen recording / screenshots:

Not applicable.

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • My PR is linked to a GitHub issue via Fixes #<issue-number> above.
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.
  • For UI changes: I attached a screen recording and/or screenshots above.
  • I have added tests (unit / integration / Playwright as applicable) and listed them above.

Summary by Gitar

  • Testing:
    • Unskipped the bulk import Playwright test suite in BulkImport.spec.ts
  • Bug fix:
    • Updated editTaskComment to send application/json content type to prevent 415 errors

This will update automatically on new commits.

Greptile Summary

The PR enables the existing Bulk Import Export Playwright suite by replacing test.describe.fixme with test.describe.

Confidence Score: 4/5

The PR is not yet safe to merge because each newly enabled suite run permanently leaks its shared catalog fixtures.

The enabled suite creates six persistent entities in beforeAll without suite-level teardown, so repeated executions accumulate catalog and search state even after the previously reported table cleanup is added.

openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/BulkImport.spec.ts

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/BulkImport.spec.ts Enables the bulk-import suite, but its shared setup now runs without removing the users, glossary objects, and domains it creates.

Reviews (2): Last reviewed commit: "Merge branch 'main' into unskip-bulk-imp..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@Rohit0301 Rohit0301 self-assigned this Jul 24, 2026
@Rohit0301 Rohit0301 added the safe to test Add this label to run secure Github workflows on PRs label Jul 24, 2026
@Rohit0301
Rohit0301 requested a review from a team as a code owner July 24, 2026 10:16
@github-actions

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

};

test.describe.fixme('Bulk Import Export', () => {
test.describe('Bulk Import Export', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Enabled test leaks table entity

Enabling this suite also runs the Table test, which creates tableEntity but only disposes the API context without deleting the entity. This leaves catalog data behind, accumulating state across runs and reducing test isolation.

@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

🔴 Playwright Results — workflow failed

Validated commit d852beb7e42c12c0c130020433a537cc78ca35ba in Playwright run 30088208145, attempt 1.

✅ 0 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Pipeline and setup failures (2)

  • Shard chromium-01 did not upload a usable Playwright results artifact.
  • Shard chromium-01 did not upload its execution status.

Performance

Blocking targets: ❌ unmet · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 1h 4m 49s

⏱️ Max setup 1m 23s · max shard execution 20m 2s · max shard-job elapsed before upload 23m 25s · reporting 5s

🌐 162.50 requests/attempt · 1.30 app boots/UI scenario · 0.00% common-shard skew

Optimization targets still in progress:

  • Application boot ratio was 1.3 per UI scenario (70 boots / 54 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
⛔ Shard chromium-01

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

@gitar-bot

gitar-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Unskips the bulk import Playwright E2E test to re-enable coverage for the feature. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

};

test.describe.fixme('Bulk Import Export', () => {
test.describe('Bulk Import Export', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Shared setup leaks catalog fixtures

When the newly enabled suite runs, beforeAll creates two users, a glossary, a glossary term, and two domains without any suite-level teardown, causing every run to leave six persistent entities in the catalog and search state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant