Skip to content

fix(test): increase timeout for platformLineage for AUTs#24973

Merged
chirag-madlani merged 3 commits intomainfrom
fix-platform-spec
Dec 23, 2025
Merged

fix(test): increase timeout for platformLineage for AUTs#24973
chirag-madlani merged 3 commits intomainfrom
fix-platform-spec

Conversation

@chirag-madlani
Copy link
Copy Markdown
Collaborator

@chirag-madlani chirag-madlani commented Dec 23, 2025

This pull request introduces a minor update to the PlatformLineage.spec.ts Playwright test. The main change is the addition of a condition to mark the test as "slow" when not running in open-source (OSS) mode. This helps ensure the test runner allocates more time for completion in certain environments.

  • Testing improvements:
    • Marked the Verify Platform Lineage View test as slow if the environment is not OSS, using test.slow(process.env.PLAYWRIGHT_IS_OSS === false).

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

  • Test configuration:
    • Added conditional timeout increase using test.slow() for Platform Lineage E2E test in non-OSS environments

This will update automatically on new commits.


@gitar-bot
Copy link
Copy Markdown

gitar-bot Bot commented Dec 23, 2025

Code Review ⚠️ Changes requested

Bug in environment variable comparison: string compared to boolean will never match, rendering test.slow() ineffective.

⚠️ Bug: Incorrect comparison: `process.env` value is a string, not boolean

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/PlatformLineage.spec.ts:20

The condition process.env.PLAYWRIGHT_IS_OSS === false will never be true because environment variables are always strings (or undefined), not booleans. Comparing a string to false with strict equality will always return false, making test.slow() never activate.

Impact: The test timeout extension intended for non-OSS environments will never take effect, potentially causing flaky test failures in those environments.

Suggested fix:

test.slow(process.env.PLAYWRIGHT_IS_OSS === 'false')

Or, if the intent is to slow the test when the env var is NOT set or is explicitly 'false':

test.slow(process.env.PLAYWRIGHT_IS_OSS !== 'true')
More details 💡 1 suggestion
💡 Code Quality: Missing semicolon at end of statement

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Flow/PlatformLineage.spec.ts:20

The test.slow() call is missing a semicolon at the end. While JavaScript/TypeScript ASI (Automatic Semicolon Insertion) will handle this, it's inconsistent with typical coding style and may trigger linter warnings.

Suggested fix:

test.slow(process.env.PLAYWRIGHT_IS_OSS === 'false');

Recommendations

  • Fix the type comparison: process.env.PLAYWRIGHT_IS_OSS is a string (or undefined), not a boolean. Comparing it to false with === will always evaluate to false, so test.slow() will never activate.
  • Add the missing semicolon for consistency with the codebase style.
Options

Auto-apply is off Gitar will not commit updates to this branch.
✅ Code review is on Gitar will review this change.
Display: compact Hiding non-applicable rules.

Comment with these commands to change:

Auto-apply ✅ Code review Compact
gitar auto-apply:on         
gitar code-review:off         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | This comment will update automatically (Docs)

@github-actions
Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 64%
64.31% (50914/79172) 41.94% (24808/59145) 45.4% (7813/17209)

@sonarqubecloud
Copy link
Copy Markdown

@chirag-madlani chirag-madlani merged commit a650422 into main Dec 23, 2025
23 checks passed
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 UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants