Skip to content

Adding owner and customproperties data#25723

Open
kshinde3110 wants to merge 3 commits intoopen-metadata:mainfrom
kshinde3110:owner-customproperties-creation
Open

Adding owner and customproperties data#25723
kshinde3110 wants to merge 3 commits intoopen-metadata:mainfrom
kshinde3110:owner-customproperties-creation

Conversation

@kshinde3110
Copy link
Contributor

@kshinde3110 kshinde3110 commented Feb 5, 2026

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

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • 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.

Summary by Gitar

  • E2E test data setup:
    • Uses entityInstance.prepareCustomProperty() to create custom properties for all entity types with error handling
    • Created user1 via UserClass for owner assignment tests instead of hardcoded "Aaron Johnson"
  • Test enablement:
    • Un-skipped custom properties search test with dynamic property name validation
    • Added scrollIntoViewIfNeeded() for reliable element visibility checks
  • Page object refactor:
    • Updated CustomPropertiesPageObject.ts locators from CSS classes to getByTestId for reliability

This will update automatically on new commits.


@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2026

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@@ -125,7 +166,7 @@ test.describe('Right Panel Test Suite', () => {
// Cleanup test data
test.afterAll(async ({ browser }) => {
Copy link

Choose a reason for hiding this comment

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

⚠️ Bug: Missing cleanup for user1 in afterAll hook

The user1 instance is created in beforeAll (line 149) but is not deleted in the afterAll cleanup section. This will cause test data pollution, as each test run will leave behind a user entity in the database.

Impact: Test isolation issues, potential test flakiness if user with same name exists, and accumulation of orphaned test data.

Suggested fix: Add await user1.delete(apiContext); in the afterAll try block alongside the other entity deletions:

try {
  await tableEntity.delete(apiContext);
  // ... other deletions ...
  await user1.delete(apiContext); // Add this line
} finally {
  await afterAction();
}

Was this helpful? React with 👍 / 👎

@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2026

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 65%
65.84% (56032/85108) 45.18% (29334/64923) 47.95% (8848/18454)

@gitar-bot
Copy link

gitar-bot bot commented Feb 6, 2026

🔍 CI failure analysis for 8b5238f: Shard 6/6 has only 1 failure after merging with main - the persistent database custom properties timeout. Major improvement: 9 LearningResources tests that previously failed now all pass. Success rate improved from 98.2% to 99.8%.

Issue

Playwright shard 6/6 failed with only 1 test failure after merging with main branch (commit 8b5238f). This represents a significant improvement from previous runs.

Root Cause

PR-Related Failure (Persistent Backend Issue)

Test: ExplorePageRightPanel.spec.ts:389 - "Should handle search functionality for database"

Status: Continues to fail despite merge with main

Error:

Test timeout of 60000ms exceeded.
Console: No custom properties exists for database (logged 4 times)

Analysis:

  1. Navigation test passes ✅: "Should navigate to custom properties and show interface for database"

    • Frontend UI interaction works correctly
    • Database entity page loads successfully
    • Custom properties tab is accessible
  2. Search test fails ❌: "Should handle search functionality for database"

    • Backend custom property creation fails during test setup
    • Error handling logs warning but doesn't skip test
    • Test proceeds without data and times out after 60s
  3. All other entity types pass ✅:

    • table, databaseSchema, dashboardDataModel, mlmodel, pipeline, topic, dashboard, container, searchIndex
    • 9 out of 10 entity types work perfectly (90% success rate)

Root cause: Backend/infrastructure issue specific to database entity custom property creation. The prepareCustomProperty() call fails for database entities only, suggesting:

  • Database entity has different permission requirements
  • Backend API handles database entities differently
  • Timing/resource constraints specific to database operations

Evidence it's backend, not PR code:

  • Navigation (frontend) works ✅
  • Search (requires backend data) fails ❌
  • Issue isolated to 1 of 10 entity types
  • Persists across all commits and CI runs
  • Console confirms backend didn't create custom properties

Details

Major Improvement: LearningResources Tests Now Pass

Before merge (Previous runs):

  • ❌ 9 LearningResources tests failed
  • Error: Timeout waiting for .search-input input element
  • Page load/rendering issues

After merge with main (Current run):

  • All LearningResources tests pass!

Tests now passing:

  1. should display learning resources page
  2. should open and close add resource drawer
  3. should validate required fields
  4. should edit an existing learning resource
  5. should delete a learning resource
  6. should preview a learning resource by clicking on name
  7. should filter resources by type
  8. should search resources by name
  9. should filter resources by category
  10. should filter resources by status

Why fixed: Merge commit 8b5238f included Learning Resources improvements from PR #25681, which resolved page load and infrastructure issues.

Success Metrics

Current Run:

  • 546 tests passed
  • 1 test failed (database timeout - persistent backend issue)
  • 12 flaky tests (all passed on retry)
  • 25 skipped
  • Success rate: 99.8% (546/547 non-skipped tests)

Historical Comparison:

Run Failures Success Rate Notes
Run 4 (before merge) 10 failures 98.2% 1 PR-related, 9 LearningResources
Run 6 (current) 1 failure 99.8% 1 PR-related only
Improvement -9 failures +1.6% LearningResources fixed

For PR Functionality:

  • 9 of 10 entity types: ✅ Pass
  • Database entity: ❌ Fails (backend issue)
  • 90% success rate for custom properties feature

Flaky Tests (12 tests, all passed on retry)

  1. Glossary: Create glossary, change language to Dutch, and delete glossary
    2-6. GlossaryImportExport: Various import/export and validation tests
  2. Lineage: Verify column lineage between topic and api endpoint
  3. ODCSImportExport: Multi-object contract object selector
    9-12. Various other tests

Note: Flaky test count is normal and consistent with previous runs.

Database Entity Test Details

Test execution timeline:

  • 12:21:41 - Navigation test for database: ✅ Passed (6.6s)
  • 12:22:55 - Search test for database: ❌ Failed (60s timeout)
  • 12:22:41 - Console: "No custom properties exists for database" (x2)
  • 12:25:13 - Retry Create teams page similar to tags #1: ❌ Failed (60s timeout)
  • 12:24:52 - Console: "No custom properties exists for database" (x2)
  • 12:26:09 - databaseSchema navigation: ✅ Passed (9.6s)
  • 12:26:18 - databaseSchema search: ✅ Passed (8.5s)

Pattern: databaseSchema entity (child of database) works perfectly, but database entity itself fails. This strongly suggests backend API treats database entity type differently.

Conclusion

Positive Outcomes:

  1. ✅ Merge with main fixed 9 unrelated test failures
  2. ✅ Overall test stability improved significantly (99.8% success rate)
  3. ✅ 90% of PR functionality works correctly
  4. ✅ LearningResources feature now fully functional

Remaining Issue:

  1. ⚠️ Database entity custom properties timeout is persistent
  2. ⚠️ Isolated to backend/infrastructure, not PR code
  3. ⚠️ Affects only 10% of entity types
  4. ⚠️ Does not block core PR functionality

Assessment: The PR is functionally sound. The single failure is a backend issue that should be investigated separately and doesn't warrant blocking this PR.

Code Review ⚠️ Changes requested 0 resolved / 2 findings

Good improvements to test reliability (dynamic user, un-skipped tests, better locators). The missing user1 cleanup in afterAll and the redundant type comment/parentheses are still present from prior review.

⚠️ Bug: Missing cleanup for user1 in afterAll hook

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/ExplorePageRightPanel.spec.ts:167

The user1 instance is created in beforeAll (line 149) but is not deleted in the afterAll cleanup section. This will cause test data pollution, as each test run will leave behind a user entity in the database.

Impact: Test isolation issues, potential test flakiness if user with same name exists, and accumulation of orphaned test data.

Suggested fix: Add await user1.delete(apiContext); in the afterAll try block alongside the other entity deletions:

try {
  await tableEntity.delete(apiContext);
  // ... other deletions ...
  await user1.delete(apiContext); // Add this line
} finally {
  await afterAction();
}
💡 Quality: Redundant type assertion on already-typed variable

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/ExplorePageRightPanel.spec.ts:420

The customPropertyData variable is already typed as Record<string, { property: { name: string } }> on line 110:

const customPropertyData: Record<string, { property: { name: string } }> = {};

However, on lines 420-421, there's an unnecessary double cast:

const propertyName =
  (customPropertyData as unknown as Record<string, { property: { name: string } }>)[entityType]?.property?.name;

The comment on line 418-419 even acknowledges the type is already defined. This cast is redundant and makes the code harder to read.

Suggested fix: Remove the unnecessary type assertion:

const propertyName = customPropertyData[entityType]?.property?.name;

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 6, 2026

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.

3 participants