Fix: Flaky Workflow Test cross polluted by SystemResource test#25789
Fix: Flaky Workflow Test cross polluted by SystemResource test#25789
Conversation
🔍 CI failure analysis for 0c617ff: CI failures span backend integration tests (search indexing timeouts) and frontend Playwright tests (UI timing issues). All failures are unrelated to this PR's AssetCertificationSettings cleanup changes.IssueThe CI failures across multiple test suites are pre-existing flaky tests unrelated to this PR's changes. This PR only modifies test cleanup logic for Root CauseTwo Categories of Unrelated Failures: 1. Backend Integration Tests: Search Indexing Infrastructure IssuesAll failing backend tests involve Elasticsearch/OpenSearch search indexing operations experiencing timing issues and race conditions:
2. Frontend E2E Tests: Playwright Browser/Timing IssuesPlaywright tests failing due to UI timing and browser stability issues: Hard Failures:
Flaky Tests (8 passed on retry):
DetailsInfrastructure Problems:Elasticsearch/OpenSearch Shard Failures: Recurring OpenSearch Mapping Exception: Search index schema mismatch - expects keyword string but receives object. Thread Interruption: Event processing threads forcefully interrupted during shutdown (LMAX Disruptor cleanup issues) Browser Instability: Playwright tests experiencing unexpected page closures and element rendering delays in CI environment Why Unrelated to This PR:
Code Review 👍 Approved with suggestions 0 resolved / 1 findingsGood fix for cross-test pollution with a two-pronged approach (source cleanup + defensive initialization). One minor suggestion to use try/finally for more robust cleanup. 💡 Edge Case: Reset in SystemResourceIT skipped if assertions fail📄 openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/SystemResourceIT.java:1031 The cleanup code that resets A more robust pattern would wrap the test logic in a try {
// ... update settings and assert ...
assertEquals("NewCertification", updatedCertificationConfig.getAllowedClassification());
assertEquals("P60D", updatedCertificationConfig.getValidityPeriod());
} finally {
// Reset to original values to avoid cross-test pollution
certificationConfig.setAllowedClassification("Certification");
certificationConfig.setValidityPeriod("P30D");
// ... PUT reset settings ...
}This is minor because the defensive setup in Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
|
Failed to cherry-pick changes to the 1.11.9 branch. |
Describe your changes:
Fixes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>Summary by Gitar
test_assetCertificationSettingsinSystemResourceIT.javanow resetsAssetCertificationSettingsto defaults after test completionsetupCertificationTags_SDKinWorkflowDefinitionResourceIT.javaensuresCertification.Goldtag exists and resets settings before workflow testsThis will update automatically on new commits.