Skip to content

Conversation

@TeddyCr
Copy link
Collaborator

@TeddyCr TeddyCr commented Dec 22, 2025

…testSuite pipeline

Describe your changes:

Fixes #24958

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

  • Fixed alert filtering bug:
    • AlertsRuleEvaluator.matchTestResult() now correctly handles TEST_SUITE entities instead of triggering all TestSuite alerts
  • Extended entity type checking:
    • Added TEST_SUITE to entity type validation in matchTestResult() method
  • New field handling:
    • Parse testCaseResultSummary field changes and check ResultSummary status values against configured alert filters

This will update automatically on new commits.


mohityadav766
mohityadav766 previously approved these changes Dec 22, 2025
@TeddyCr
Copy link
Collaborator Author

TeddyCr commented Dec 22, 2025

gitar auto-apply:on

@github-actions
Copy link
Contributor

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

@sonarqubecloud
Copy link

@TeddyCr TeddyCr merged commit 02ce77e into open-metadata:main Dec 23, 2025
19 of 21 checks passed
@gitar-bot
Copy link

gitar-bot bot commented Dec 23, 2025

Code Review ⚠️ Changes requested

Good extension of the matchTestResult filter to support TEST_SUITE entities, with one null safety concern.

⚠️ Bug: Potential NPE when accessing resultSummary.getStatus()

📄 openmetadata-service/src/main/java/org/openmetadata/service/events/subscription/AlertsRuleEvaluator.java:42

The code calls resultSummary.getStatus().value() without checking if getStatus() returns null. If a ResultSummary object has a null status, this will throw a NullPointerException.

Impact: This could cause the alert evaluation to fail unexpectedly when processing a test suite with incomplete result summaries, potentially missing alerts or causing subscription processing errors.

Suggested fix:

for (ResultSummary resultSummary : resultSummaries) {
  if (resultSummary.getStatus() != null 
      && testResults.contains(resultSummary.getStatus().value())) {
    return true;
  }
}

What Works Well

  • Clean extension of existing functionality following the established patterns
  • Proper null check for fieldChange.getNewValue() before processing
  • Logical separation of the new field handling block

Recommendations

  • Add null check for resultSummary.getStatus() to prevent potential NPE
  • Consider adding unit tests for the new TEST_SUITE handling path if not already covered
Options ✅ Auto-apply

✅ Auto-apply is on Gitar will 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:off         
gitar code-review:off         
gitar display:verbose         

This comment will update automatically (Docs)

ShaileshParmar11 pushed a commit that referenced this pull request Dec 26, 2025
* fix: handled testSuite change event when processing testResults from testSuite pipeline

* fix: handled potential null pointer errros

* fix: handled potential null pointer errros
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ingestion 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.

testSuite Alerts are always triggered

2 participants