Skip to content

[#10193] test(optimizer): add optimizer integration tests#10191

Open
FANNG1 wants to merge 4 commits intoapache:mainfrom
FANNG1:add-it
Open

[#10193] test(optimizer): add optimizer integration tests#10191
FANNG1 wants to merge 4 commits intoapache:mainfrom
FANNG1:add-it

Conversation

@FANNG1
Copy link
Contributor

@FANNG1 FANNG1 commented Mar 4, 2026 •
edited by jerryshao
Loading

What changes were proposed in this pull request?

  • Ported optimizer integration tests from recommender to add-it and adapted
    them to the current APIs/configs.
  • Updated IT environment setup to create tables via lakehouse-iceberg catalog
    (memory backend) instead of relying on legacy reflective/internal paths.
  • Reworked RecommenderIT to use public recommender APIs and a test recording
    JobSubmitter (session-isolated) instead of reflection.
  • Removed remaining reflection-based field injection in updater unit tests by
    adding package-private test hooks in:
    • GravitinoMetricsUpdater
    • GravitinoStatisticsUpdater
  • Added/updated related test service registration and assertions accordingly.

Why are the changes needed?

  • add-it branch has API/config differences from recommender; direct copy of
    ITs is not compatible.
  • Reflection-based tests are brittle and tightly coupled to internals, causing
    maintenance and stability issues.
  • Session-isolated recording in IT avoids shared static-state interference and
    makes tests safer for parallel runs.

Fix: #10193

Does this PR introduce any user-facing change?

  • No user-facing API changes.
  • No new user-facing property keys for production behavior.
  • Changes are limited to test code/test wiring and testability hooks.

How was this patch tested?

  • Added/updated integration and unit tests to cover migrated paths and
    reflection removal.
  • Ran formatting:
    • ./gradlew :maintenance:optimizer:spotlessApply
  • Ran targeted tests (with required env vars):
    • :maintenance:optimizer:test --tests
      'org.apache.gravitino.maintenance.optimizer.integration.test.Recommender
      IT' -PskipIT=false -PskipDockerTests=false
    • :maintenance:optimizer:test --tests
      'org.apache.gravitino.maintenance.optimizer.integration.test.*' (targete
      d migrated IT classes)
    • :maintenance:optimizer:test --tests
      'org.apache.gravitino.maintenance.optimizer.updater.statistics.TestGravi
      tinoStatisticsUpdater' --tests
      'org.apache.gravitino.maintenance.optimizer.updater.metrics.TestGravitin
      oMetricsUpdater' -PskipIT=true -PskipDockerTests=false
  • Result: targeted IT/UT passed.

FANNG1 added 2 commits March 4, 2026 16:36
@FANNG1
test: port optimizer integration tests to add-it branch
c0cd08e 
Copy optimizer integration tests from recommender and adapt to add-it APIs/configuration changes.

Handle branch difference by creating test tables through lakehouse-iceberg catalog instead of registering format=iceberg on generic catalog.

Add required test dependencies and service registrations for dummy statistics calculators.
@FANNG1
test: remove reflective access and stabilize optimizer tests
d5149e0 
Remove reflection-based field/method access from optimizer tests by adding package-private test hooks.

Stabilize RecommenderIT with session-isolated recording submitter and unbounded limit for recommendation capture.
@github-actions
Copy link

github-actions bot commented Mar 4, 2026 •
edited
Loading

Code Coverage Report

Overall Project 65.19% +0.03% 🟢
Files changed 90.0% 🟢

Module Coverage
aliyun 1.73% 🔴
api 46.15% 🟢
authorization-common 85.96% 🟢
aws 1.1% 🔴
azure 2.6% 🔴
catalog-common 10.0% 🔴
catalog-fileset 80.02% 🟢
catalog-hive 80.98% 🟢
catalog-jdbc-clickhouse 77.71% 🟢
catalog-jdbc-common 36.84% 🔴
catalog-jdbc-doris 80.28% 🟢
catalog-jdbc-hologres 57.71% 🟢
catalog-jdbc-mysql 79.23% 🟢
catalog-jdbc-oceanbase 78.38% 🟢
catalog-jdbc-postgresql 81.22% 🟢
catalog-jdbc-starrocks 78.27% 🟢
catalog-kafka 77.01% 🟢
catalog-lakehouse-generic 45.07% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 87.15% 🟢
catalog-lakehouse-paimon 77.71% 🟢
catalog-model 77.72% 🟢
cli 44.51% 🟢
client-java 77.73% 🟢
common 49.2% 🟢
core 81.19% 🟢
filesystem-hadoop3 76.97% 🟢
flink 38.86% 🔴
flink-runtime 0.0% 🔴
gcp 14.2% 🔴
hadoop-common 10.39% 🔴
hive-metastore-common 45.82% 🟢
iceberg-common 50.21% 🟢
iceberg-rest-server 66.24% 🟢
integration-test-common 0.0% 🔴
jobs 62.55% 🟢
lance-common 23.78% 🔴
lance-rest-server 57.84% 🟢
lineage 53.02% 🟢
optimizer 82.64% +0.21% 🟢
server 85.76% 🟢
server-common 68.6% 🟢
spark 35.09% 🔴
spark-common 39.88% 🔴
trino-connector 31.62% 🔴
Files
Module File Coverage
optimizer GravitinoMetricsUpdater.java 100.0% 🟢
GravitinoStatisticsUpdater.java 85.0% 🟢

@FANNG1 FANNG1 changed the title test: port optimizer integration tests to add-it branch [#10193] test(optimizer): add optimizer integration tests Mar 4, 2026
@FANNG1 FANNG1 added the branch-1.2 Automatically cherry-pick commit to branch-1.2 label Mar 4, 2026
@FANNG1 FANNG1 requested a review from Copilot March 4, 2026 09:56
Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR ports and adapts optimizer integration tests into the maintenance/optimizer module, replaces reflection-based test injection with package-private test hooks, and wires up additional test-only SPI providers/calculators via META-INF/services.

Changes:

  • Added multiple optimizer integration tests (recommender/monitor/updater/providers) with a shared GravitinoOptimizerEnvIT environment.
  • Removed reflection-based field injection from updater unit tests by adding package-private test hooks in GravitinoMetricsUpdater and GravitinoStatisticsUpdater.
  • Registered additional test implementations via META-INF/services and updated optimizer test dependencies.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
maintenance/optimizer/src/test/resources/META-INF/services/org.apache.gravitino.maintenance.optimizer.api.updater.StatisticsCalculator Registers dummy statistics/metrics calculators for integration tests.
maintenance/optimizer/src/test/resources/META-INF/services/org.apache.gravitino.maintenance.optimizer.api.common.Provider Registers the recording JobSubmitter test provider.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/updater/statistics/TestGravitinoStatisticsUpdater.java Switches from reflection to test hook for injecting GravitinoClient.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/updater/metrics/TestGravitinoMetricsUpdater.java Switches from reflection to test hooks for injecting/accessing MetricsRepository.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/GravitinoOptimizerEnvIT.java New shared IT base that boots Gravitino server + Iceberg catalog + optimizer env.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/RecommenderIT.java New recommender IT using public APIs and a recording JobSubmitter.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/UpdaterIT.java New updater IT validating table stats/metrics and job metrics flows end-to-end.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/MonitorIT.java New monitor IT validating evaluator outputs and callbacks.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/GravitinoStatisticsIT.java New IT verifying stats updater/provider behavior.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/GravitinoMetricsIT.java New IT verifying metrics updater/provider behavior.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/GravitinoStrategyIT.java New IT verifying strategy provider reads policies correctly.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/GravitinoTableMetaIT.java New IT verifying table metadata provider behavior.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/RecordingJobSubmitterForIT.java New session-isolated recording submitter used by recommender ITs.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/DummyTableStatisticsComputer.java New dummy calculator for table stats + metrics.
maintenance/optimizer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/DummyJobMetricsComputer.java New dummy calculator for job metrics.
maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/updater/statistics/GravitinoStatisticsUpdater.java Adds package-private test hook to inject GravitinoClient.
maintenance/optimizer/src/main/java/org/apache/gravitino/maintenance/optimizer/updater/metrics/GravitinoMetricsUpdater.java Adds package-private test hooks to inject/access MetricsRepository.
maintenance/optimizer/build.gradle.kts Adds test dependencies needed for new integration tests (BaseIT/server).

@FANNG1
test: fix optimizer IT resource cleanup and H2 isolation
536f45c 
Add missing @afterall cleanup hooks in optimizer integration tests to close updater/provider resources initialized in @BeforeAll.

Replace hard-coded /tmp H2 file URLs with isolated in-memory H2 JDBC URLs to avoid filesystem residue and cross-run interference.
@FANNG1 FANNG1 self-assigned this Mar 4, 2026
@FANNG1 FANNG1 requested a review from Copilot March 4, 2026 12:00
Copilot AI reviewed Mar 4, 2026
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

jerryshao
jerryshao reviewed Mar 4, 2026
View reviewed changes
...apache/gravitino/maintenance/optimizer/integration/test/AbstractGravitinoOptimizerEnvIT.java
IcebergConstants.WAREHOUSE,
"file:///tmp/gravitino-optimizer/"));
}
}
Copy link
Contributor

@jerryshao jerryshao Mar 4, 2026

Choose a reason for hiding this comment

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

You called this class xxxIT, but I don't find any test in this class, is it on purpose?

Copy link
Contributor Author

@FANNG1 FANNG1 Mar 4, 2026

Choose a reason for hiding this comment

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

this class is used to setup enviroment , rename to AbstractGravitinoOptimizerEnvIT and make it abstract

.../org/apache/gravitino/maintenance/optimizer/integration/test/RecordingJobSubmitterForIT.java

@Override
public void close() throws Exception {}
}
Copy link
Contributor

@jerryshao jerryshao Mar 4, 2026

Choose a reason for hiding this comment

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

Also here, what do you test here?

Copy link
Contributor Author

@FANNG1 FANNG1 Mar 4, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This class is used for testing, to record job submission context

...zer/src/test/java/org/apache/gravitino/maintenance/optimizer/integration/test/UpdaterIT.java
Assertions.assertTrue(diff >= 0 && diff <= 10000);
Assertions.assertEquals(1L, ((Number) jobMetric.value().value()).longValue());
}
}
Copy link
Contributor

@jerryshao jerryshao Mar 4, 2026

Choose a reason for hiding this comment

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

Can we have a IT that can try to simulate the completed workflow of table maintenance and verify each step?

Copy link
Contributor Author

@FANNG1 FANNG1 Mar 4, 2026

Choose a reason for hiding this comment

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

RecommenderIT covers the workflow from updating stats, attaching policy, recommend for table and submit job to RecordingJobSubmitterForTest

@FANNG1
test: align optimizer IT naming and test-only hooks
dd94f65 
Annotate test hook methods in metrics/statistics updaters with @VisibleForTesting to clarify non-production usage.

Rename the shared IT base to AbstractGravitinoOptimizerEnvIT and update inheriting integration tests.

Rename dummy job metrics computer to calculator, adjust partition helper naming, and update related IT/service references.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jerryshao jerryshao jerryshao left review comments

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@FANNG1 FANNG1

Labels

branch-1.2 Automatically cherry-pick commit to branch-1.2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Subtask] add optimizer integration test

3 participants

@FANNG1 @jerryshao