Skip to content

Commit 5e3507d

Browse files
pmbrullpmbrull
authored
Merge branch 'main' into fix/k8s-secrets-manager-provider
2 parents d60132a + bebcef6 commit 5e3507d

File tree

65 files changed

+3693
-376
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

65 files changed

+3693
-376
lines changed

‎.claude/skills/playwright-validation/SKILL.md‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
---
2+
name: playwright-validation
3+
description: Use when validating UI changes in a branch require Playwright E2E testing. Reviews branch changes, validates UI with Playwright MCP, and adds missing test cases.
4+
---
5+
6+
# Playwright Validation Skill
7+
8+
This skill guides you through validating UI changes and ensuring comprehensive Playwright E2E test coverage.
9+
10+
## When to Use
11+
12+
- After completing UI feature development
13+
- Before creating a PR for UI changes
14+
- When reviewing UI-related branches
15+
- To verify existing Playwright tests cover all scenarios
16+
17+
## Workflow
18+
19+
### Phase 1: Review Branch Changes
20+
21+
1. **Identify changed files vs main:**
22+
```bash
23+
git diff main --stat
24+
git diff main --name-only | grep -E "\.(tsx?|less|css|scss)$"
25+
```
26+
27+
2. **Focus on UI component changes:**
28+
```bash
29+
git diff main -- "openmetadata-ui/src/main/resources/ui/src/components/**" --stat
30+
```
31+
32+
3. **Check for existing Playwright tests:**
33+
```bash
34+
git diff main --name-only | grep -E "playwright.*\.spec\.ts$"
35+
```
36+
37+
4. **Read the changed component files** to understand the UI modifications
38+
39+
### Phase 2: Review Existing Playwright Tests
40+
41+
1. **Locate relevant test files:**
42+
- Check `playwright/e2e/Pages/` for page-level tests
43+
- Check `playwright/e2e/Features/` for feature-specific tests
44+
- Use Glob/Grep to find tests related to the feature
45+
46+
2. **Analyze test coverage:**
47+
- Read the existing test file(s)
48+
- Identify the test scenarios already covered
49+
- Note any gaps in coverage based on the UI changes
50+
51+
3. **Review test utilities:**
52+
- Check `playwright/utils/` for helper functions
53+
- Check `playwright/support/` for entity classes and fixtures
54+
55+
### Phase 3: Validate with Playwright MCP
56+
57+
1. **Start the browser and navigate:**
58+
```
59+
mcp__playwright__browser_navigate to http://localhost:8585
60+
```
61+
62+
2. **Authenticate if needed:**
63+
- Use `mcp__playwright__browser_fill_form` for login
64+
- Default admin: `admin@open-metadata.org` / `admin`
65+
66+
3. **Navigate to the feature area:**
67+
- Use `mcp__playwright__browser_click` for navigation
68+
- Use `mcp__playwright__browser_snapshot` to inspect page state
69+
70+
4. **Validate UI behavior:**
71+
- Test the main user flows
72+
- Verify visual elements (icons, badges, labels)
73+
- Check interactive elements (buttons, dropdowns, forms)
74+
- Verify state changes and API calls
75+
76+
5. **Document findings:**
77+
- Note what works correctly
78+
- Identify any issues or missing functionality
79+
- List scenarios not covered by existing tests
80+
81+
### Phase 4: Add Missing Test Cases
82+
83+
1. **Create a TodoWrite checklist** of missing test scenarios
84+
85+
2. **For each missing test case:**
86+
87+
a. **Add necessary test fixtures** in `beforeAll`:
88+
- Create new entity instances (TableClass, DataProduct, etc.)
89+
- Set up required relationships (domains, assets)
90+
91+
b. **Add cleanup** in `afterAll`:
92+
- Delete created entities in reverse order
93+
94+
c. **Write the test** following the pattern:
95+
```typescript
96+
test('Descriptive Test Name - What it validates', async ({ page }) => {
97+
test.setTimeout(300000);
98+
99+
await test.step('Step description', async () => {
100+
// Test actions and assertions
101+
});
102+
103+
await test.step('Next step', async () => {
104+
// More actions and assertions
105+
});
106+
});
107+
```
108+
109+
3. **Test patterns to cover:**
110+
- Happy path (expected behavior)
111+
- Edge cases (empty states, max values)
112+
- Error handling (invalid inputs, failed requests)
113+
- State transitions (before/after actions)
114+
- UI feedback (loading states, success/error messages)
115+
- Permissions (disabled buttons, restricted actions)
116+
117+
4. **Run lint check:**
118+
```bash
119+
yarn eslint playwright/e2e/Pages/YourTest.spec.ts
120+
```
121+
122+
## Common Test Utilities
123+
124+
### Navigation
125+
```typescript
126+
import { sidebarClick } from '../../utils/sidebar';
127+
import { redirectToHomePage } from '../../utils/common';
128+
import { selectDataProduct, selectDomain } from '../../utils/domain';
129+
```
130+
131+
### Waiting
132+
```typescript
133+
import { waitForAllLoadersToDisappear } from '../../utils/entity';
134+
await page.waitForLoadState('networkidle');
135+
await page.waitForSelector('[data-testid="loader"]', { state: 'detached' });
136+
```
137+
138+
### API Responses
139+
```typescript
140+
const response = page.waitForResponse('/api/v1/endpoint*');
141+
await someAction();
142+
await response;
143+
expect((await response).status()).toBe(200);
144+
```
145+
146+
### Assertions
147+
```typescript
148+
await expect(page.getByTestId('element')).toBeVisible();
149+
await expect(page.getByTestId('element')).toContainText('text');
150+
await expect(page.locator('.class')).not.toBeVisible();
151+
```
152+
153+
## Checklist Before Completion
154+
155+
- [ ] All UI changes have corresponding test coverage
156+
- [ ] Tests cover both positive and negative scenarios
157+
- [ ] Tests verify visual indicators (icons, badges, states)
158+
- [ ] Tests validate API interactions
159+
- [ ] Lint check passes with no errors
160+
- [ ] Test fixtures are properly created and cleaned up
161+
- [ ] Test timeouts are appropriate (300000ms for complex tests)
162+
163+
## Example: Data Contract Inheritance Tests
164+
165+
For reference, see the comprehensive test coverage in:
166+
`playwright/e2e/Pages/DataContractInheritance.spec.ts`
167+
168+
This file demonstrates:
169+
- Multiple entity setup in beforeAll
170+
- Domain assignment patches
171+
- Contract creation and validation
172+
- Inheritance icon verification
173+
- Action button state verification (disabled/enabled)
174+
- API response validation (POST vs PATCH)
175+
- Fallback behavior testing

‎.claude/skills/writing-playwright-tests/SKILL.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
name: writing-playwright-tests
3+
description: Use when writing new Playwright E2E tests or adding test cases. Provides testing philosophy, patterns, and best practices from the Playwright Developer Handbook.
4+
---
5+
6+
# Writing Playwright Tests Skill
7+
8+
This skill guides you through writing Playwright E2E tests following OpenMetadata standards.
9+
10+
**Reference**: @openmetadata-ui/src/main/resources/ui/playwright/PLAYWRIGHT_DEVELOPER_HANDBOOK.md

‎ingestion/src/metadata/ingestion/models/patch_request.py‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -416,15 +416,6 @@ def build_patch(
416416
updated_operations = JsonPatchUpdater.from_restrict_update_fields(
417417
restrict_update_fields
418418
).update(patch, override_metadata=override_metadata)
419-
420-
# if the only operation is to update the sourceHash, we'll skip the patch
421-
# since this will only happen when we filter out the REMOVE and REPLACE ops
422-
# and if the sourceHash is updated in this case further updates will not be processed
423-
if (
424-
len(updated_operations) == 1
425-
and updated_operations[0].get("path") == "/sourceHash"
426-
):
427-
return None
428419
patch.patch = updated_operations
429420

430421
return patch

‎ingestion/src/metadata/ingestion/source/dashboard/metabase/models.py‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import json
1616
from typing import List, Optional
1717

18-
from pydantic import BaseModel, BeforeValidator, Field, field_validator
18+
from pydantic import BaseModel, BeforeValidator, Field, field_validator, model_validator
1919
from typing_extensions import Annotated
2020

2121
MetabaseStrId = Annotated[str, BeforeValidator(lambda x: str(x))]
@@ -68,9 +68,30 @@ class Native(BaseModel):
6868

6969

7070
class DatasetQuery(BaseModel):
71+
model_config = {"extra": "ignore"}
72+
7173
type: Optional[str] = None
7274
native: Optional[Native] = None
7375

76+
@model_validator(mode="before")
77+
@classmethod
78+
def normalize_native_from_stages(cls, data):
79+
"""
80+
Breaking change in metabase 0.57.0
81+
https://www.metabase.com/docs/latest/developers-guide/api-changelog#metabase-0570
82+
"""
83+
if not isinstance(data, dict):
84+
return data
85+
if data.get("native") is not None:
86+
return data
87+
stages = data.get("stages")
88+
if stages:
89+
for stage in stages:
90+
if isinstance(stage, dict) and stage.get("native"):
91+
data["native"] = {"query": stage["native"]}
92+
break
93+
return data
94+
7495

7596
class MetabaseChart(BaseModel):
7697
"""

‎ingestion/tests/unit/topology/dashboard/test_metabase.py‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -429,3 +429,38 @@ def test_dataset_query_string_parsing(self):
429429
name="test_chart_none_value", id="105", dataset_query=None
430430
)
431431
self.assertIsNone(chart_with_none_value.dataset_query)
432+
433+
# Test 7: New Metabase format with stages array
434+
chart_with_stages = MetabaseChart(
435+
name="test_chart_stages",
436+
id="106",
437+
dataset_query={
438+
"lib/type": "mbql/query",
439+
"database": 2,
440+
"stages": [
441+
{
442+
"lib/type": "mbql.stage/native",
443+
"native": "SELECT * FROM new_format_table",
444+
}
445+
],
446+
},
447+
)
448+
self.assertIsNotNone(chart_with_stages.dataset_query)
449+
self.assertIsNotNone(chart_with_stages.dataset_query.native)
450+
self.assertEqual(
451+
chart_with_stages.dataset_query.native.query,
452+
"SELECT * FROM new_format_table",
453+
)
454+
455+
# Test 8: New format with stages but no native query
456+
chart_with_empty_stages = MetabaseChart(
457+
name="test_chart_empty_stages",
458+
id="107",
459+
dataset_query={
460+
"lib/type": "mbql/query",
461+
"database": 2,
462+
"stages": [{"lib/type": "mbql.stage/mbql"}],
463+
},
464+
)
465+
self.assertIsNotNone(chart_with_empty_stages.dataset_query)
466+
self.assertIsNone(chart_with_empty_stages.dataset_query.native)

‎openmetadata-integration-tests/pom.xml‎

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -148,72 +148,6 @@
148148
<scope>test</scope>
149149
</dependency>
150150
</dependencies>
151-
<build>
152-
<plugins>
153-
<plugin>
154-
<groupId>org.apache.maven.plugins</groupId>
155-
<artifactId>maven-failsafe-plugin</artifactId>
156-
<version>${maven.failsafe.version}</version>
157-
<executions>
158-
<!-- Phase 1: Run TagRecognizerFeedbackIT sequentially in integration-test phase -->
159-
<execution>
160-
<id>sequential-test</id>
161-
<goals>
162-
<goal>integration-test</goal>
163-
</goals>
164-
<configuration>
165-
<forkCount>1</forkCount>
166-
<reuseForks>true</reuseForks>
167-
<argLine>-Xmx4096m -XX:+UseG1GC</argLine>
168-
<includes>
169-
<include>**/TagRecognizerFeedbackIT.java</include>
170-
</includes>
171-
<systemPropertyVariables>
172-
<junit.jupiter.extensions.autodetection.enabled>true</junit.jupiter.extensions.autodetection.enabled>
173-
<junit.jupiter.execution.parallel.enabled>false</junit.jupiter.execution.parallel.enabled>
174-
</systemPropertyVariables>
175-
<trimStackTrace>false</trimStackTrace>
176-
<reportFormat>plain</reportFormat>
177-
<useFile>true</useFile>
178-
</configuration>
179-
</execution>
180-
<!-- Phase 2: Run all other tests in parallel in integration-test phase -->
181-
<execution>
182-
<id>parallel-tests</id>
183-
<goals>
184-
<goal>integration-test</goal>
185-
</goals>
186-
<configuration>
187-
<forkCount>1</forkCount>
188-
<reuseForks>true</reuseForks>
189-
<argLine>-Xmx4096m -XX:+UseG1GC</argLine>
190-
<includes>
191-
<include>**/*IT.java</include>
192-
<include>**/*Test.java</include>
193-
</includes>
194-
<excludes>
195-
<exclude>**/TagRecognizerFeedbackIT.java</exclude>
196-
</excludes>
197-
<systemPropertyVariables>
198-
<junit.jupiter.extensions.autodetection.enabled>true</junit.jupiter.extensions.autodetection.enabled>
199-
<junit.jupiter.execution.parallel.enabled>true</junit.jupiter.execution.parallel.enabled>
200-
</systemPropertyVariables>
201-
<trimStackTrace>false</trimStackTrace>
202-
<reportFormat>plain</reportFormat>
203-
<useFile>true</useFile>
204-
</configuration>
205-
</execution>
206-
<!-- Verify phase: Fails the build if any tests failed -->
207-
<execution>
208-
<id>verify</id>
209-
<goals>
210-
<goal>verify</goal>
211-
</goals>
212-
</execution>
213-
</executions>
214-
</plugin>
215-
</plugins>
216-
</build>
217151
<profiles>
218152
<!-- MySQL + Elasticsearch (default) -->
219153
<profile>

0 commit comments

Comments
 (0)