Skip to content

Delete Old Integration tests and fix sonar workflow#26204

Merged
harshach merged 6 commits intomainfrom
fix_workflow
Mar 5, 2026
Merged

Delete Old Integration tests and fix sonar workflow#26204
harshach merged 6 commits intomainfrom
fix_workflow

Conversation

@harshach
Copy link
Copy Markdown
Collaborator

@harshach harshach commented Mar 3, 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

  • Removed legacy integration tests:
    • Deleted 185 test files (~150K lines) including EntityResourceTest, DataContractResourceTest, TableResourceTest, and others covering resources, search, security, workflows, and data quality
    • Removed testcontainers dependencies (PostgreSQL, MySQL, Elasticsearch, OpenSearch, MinIO, K3s, Redis modules)
    • Deleted Maven test profiles (postgres-tests, mysql-tests, postgres-redis-tests, postgres-rdf-tests, mysql-redis-tests) used for containerized integration testing
  • Fixed CI workflow:
    • Changed maven-sonar-build.yml: Maven goal from package to verify for proper static-code-analysis profile execution
  • Cleaned up test infrastructure:
    • Removed Sonar test configuration properties, testcontainers reuse config, test YAML files, and related test dependencies
    • Retained core testing capability (JUnit 5, Mockito, AssertJ) for unit tests

This will update automatically on new commits.

@@ -13,10 +13,6 @@

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Quality: Sonar test report paths removed—coverage reporting may degrade

The PR removes sonar.junit.reportPaths, sonar.coverage.jacoco.xmlReportPaths, and sonar.tests properties from openmetadata-service/pom.xml. While these aren't needed for the current -DskipTests workflow, if any other CI job or local workflow runs tests and pushes results to SonarCloud for this module, the test report and coverage data won't be picked up because these paths are no longer configured.

If the intent is that test coverage is now exclusively tracked via the openmetadata-integration-tests module, this is fine. But if SonarCloud is expected to report unit test coverage for openmetadata-service in the future (e.g., for remaining unit tests like RuleEvaluatorTest, CloudWatchEventMonitorTest, etc.), these properties should be re-added or configured at the parent POM level.

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

mohityadav766
mohityadav766 previously approved these changes Mar 4, 2026
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-params</artifactId>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Quality: Hardcoded JUnit version instead of using parent property

The newly added junit-jupiter-params dependency hardcodes version 5.9.3 instead of using the ${org.junit.jupiter.version} property defined in the parent POM. While the values currently match, this creates a maintenance risk: if the parent property is updated in the future, this dependency will be left behind, potentially causing version conflicts between JUnit modules (e.g., junit-jupiter-engine at a newer version vs junit-jupiter-params at 5.9.3).

Suggested fix:

<dependency>
  <groupId>org.junit.jupiter</groupId>
  <artifactId>junit-jupiter-params</artifactId>
  <version>${org.junit.jupiter.version}</version>
  <scope>test</scope>
</dependency>

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Mar 5, 2026

Code Review 👍 Approved with suggestions 0 resolved / 2 findings

Deletes obsolete integration tests and updates the Sonar workflow configuration. Consider using the parent POM property for the JUnit version instead of hardcoding it, and verify that removing Sonar test report paths won't degrade coverage reporting.

💡 Quality: Sonar test report paths removed—coverage reporting may degrade

📄 openmetadata-service/pom.xml:13

The PR removes sonar.junit.reportPaths, sonar.coverage.jacoco.xmlReportPaths, and sonar.tests properties from openmetadata-service/pom.xml. While these aren't needed for the current -DskipTests workflow, if any other CI job or local workflow runs tests and pushes results to SonarCloud for this module, the test report and coverage data won't be picked up because these paths are no longer configured.

If the intent is that test coverage is now exclusively tracked via the openmetadata-integration-tests module, this is fine. But if SonarCloud is expected to report unit test coverage for openmetadata-service in the future (e.g., for remaining unit tests like RuleEvaluatorTest, CloudWatchEventMonitorTest, etc.), these properties should be re-added or configured at the parent POM level.

💡 Quality: Hardcoded JUnit version instead of using parent property

📄 openmetadata-service/pom.xml:573

The newly added junit-jupiter-params dependency hardcodes version 5.9.3 instead of using the ${org.junit.jupiter.version} property defined in the parent POM. While the values currently match, this creates a maintenance risk: if the parent property is updated in the future, this dependency will be left behind, potentially causing version conflicts between JUnit modules (e.g., junit-jupiter-engine at a newer version vs junit-jupiter-params at 5.9.3).

Suggested fix
    <dependency>
      <groupId>org.junit.jupiter</groupId>
      <artifactId>junit-jupiter-params</artifactId>
      <version>${org.junit.jupiter.version}</version>
      <scope>test</scope>
    </dependency>
🤖 Prompt for agents
Code Review: Deletes obsolete integration tests and updates the Sonar workflow configuration. Consider using the parent POM property for the JUnit version instead of hardcoding it, and verify that removing Sonar test report paths won't degrade coverage reporting.

1. 💡 Quality: Sonar test report paths removed—coverage reporting may degrade
   Files: openmetadata-service/pom.xml:13

   The PR removes `sonar.junit.reportPaths`, `sonar.coverage.jacoco.xmlReportPaths`, and `sonar.tests` properties from `openmetadata-service/pom.xml`. While these aren't needed for the current `-DskipTests` workflow, if any other CI job or local workflow runs tests and pushes results to SonarCloud for this module, the test report and coverage data won't be picked up because these paths are no longer configured.
   
   If the intent is that test coverage is now exclusively tracked via the `openmetadata-integration-tests` module, this is fine. But if SonarCloud is expected to report unit test coverage for `openmetadata-service` in the future (e.g., for remaining unit tests like `RuleEvaluatorTest`, `CloudWatchEventMonitorTest`, etc.), these properties should be re-added or configured at the parent POM level.

2. 💡 Quality: Hardcoded JUnit version instead of using parent property
   Files: openmetadata-service/pom.xml:573

   The newly added `junit-jupiter-params` dependency hardcodes version `5.9.3` instead of using the `${org.junit.jupiter.version}` property defined in the parent POM. While the values currently match, this creates a maintenance risk: if the parent property is updated in the future, this dependency will be left behind, potentially causing version conflicts between JUnit modules (e.g., `junit-jupiter-engine` at a newer version vs `junit-jupiter-params` at 5.9.3).

   Suggested fix:
   <dependency>
     <groupId>org.junit.jupiter</groupId>
     <artifactId>junit-jupiter-params</artifactId>
     <version>${org.junit.jupiter.version}</version>
     <scope>test</scope>
   </dependency>

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
Copy Markdown

sonarqubecloud bot commented Mar 5, 2026

@harshach harshach merged commit e586d18 into main Mar 5, 2026
42 of 47 checks passed
@harshach harshach deleted the fix_workflow branch March 5, 2026 02:22
pmbrull added a commit that referenced this pull request Mar 5, 2026
PR #26204 removed MCP tests from the openmetadata-mcp module. This
re-creates them under openmetadata-integration-tests so they run as
part of the standard integration test suite.

Changes:
- Add McpIntegrationIT: tests MCP initialization, tools/list,
  prompts/list, and tool calls via HTTP against the /mcp endpoint
- Add McpToolsValidationIT: ordered tests validating each MCP tool
  (search, get entity, create glossary/term, patch, lineage, test
  definitions, test cases, root cause analysis, deleted field handling)
- Add McpTestUtils: shared JSON-RPC request builders for MCP tests
- Register MCP server in TestSuiteBootstrap after seed data loads
  using reflection to reset ApplicationContext and invoke
  registerMCPServer()
- Add openmetadata-mcp and assertj-core dependencies to pom.xml
- Delete .github/workflows/mcp-tests.yml (no longer needed)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
pmbrull added a commit that referenced this pull request Mar 5, 2026
* Re-enable MCP tests under openmetadata-integration-tests

PR #26204 removed MCP tests from the openmetadata-mcp module. This
re-creates them under openmetadata-integration-tests so they run as
part of the standard integration test suite.

Changes:
- Add McpIntegrationIT: tests MCP initialization, tools/list,
  prompts/list, and tool calls via HTTP against the /mcp endpoint
- Add McpToolsValidationIT: ordered tests validating each MCP tool
  (search, get entity, create glossary/term, patch, lineage, test
  definitions, test cases, root cause analysis, deleted field handling)
- Add McpTestUtils: shared JSON-RPC request builders for MCP tests
- Register MCP server in TestSuiteBootstrap after seed data loads
  using reflection to reset ApplicationContext and invoke
  registerMCPServer()
- Add openmetadata-mcp and assertj-core dependencies to pom.xml
- Delete .github/workflows/mcp-tests.yml (no longer needed)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix no-op assertion and remove blanket catch blocks in McpToolsValidationIT

- Assert search result contains expected query instead of discarding
  the boolean return value of contains()
- Remove try-catch blocks in testPatchEntityTool and
  testGetEntityLineageTool that silently swallowed all exceptions

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Fix search assertion to use substring match on entity names

Search returns entity names like "mcp_val_table" for query "mcp_val",
so the assertion must check if any returned name contains the query
string rather than requiring an exact set membership match.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* Refactor: extract McpTestBase, add ApplicationContext.reinitialize()

- Add ApplicationContext.reinitialize() to replace reflection-based
  singleton reset in TestSuiteBootstrap
- Extract shared MCP test infrastructure (HTTP client, auth, entity
  creation, SSE parsing, MCP request execution) into McpTestBase
- McpIntegrationIT and McpToolsValidationIT now extend McpTestBase,
  removing ~230 lines of duplicated boilerplate
- Fix SSE parser to concatenate all data: lines per the SSE spec

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
pmbrull added a commit that referenced this pull request Mar 12, 2026
Merge and resolve conflicts for dc-dq-execute-summary branch.

- Split data contract validation into two paths: tests with existing
  results compile data, tests without results execute via DQ pipeline
- Add dataContract field to testCase schema for bidirectional references
- Add migration to backfill dataContract references on existing test cases
- Update ContractQualityCard to use qualityValidation from contract results
  instead of fetching test suite summary separately
- Fix broken JP elasticsearch mapping JSON
- Drop deleted integration test files (removed in #26204)

Conflict resolutions:
- Migration: kept flyway migration refactor from main + added new
  migrateTestCaseDataContractReferences call
- TestCaseMapper: merged withTopDimensions from main + dataContract
  conditional from PR
- ContractDetail: kept security section from main + updated quality
  section to use qualityExpectations condition and latestContractResults

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

2 participants