test(workflow-operator): add unit test coverage for SQLSourceOpDesc schema introspection#6078
Open
aglinxinyuan wants to merge 1 commit into
Open
test(workflow-operator): add unit test coverage for SQLSourceOpDesc schema introspection#6078aglinxinyuan wants to merge 1 commit into
aglinxinyuan wants to merge 1 commit into
Conversation
…uerySchema Targets Codecov-missed lines (27) in the abstract SQL descriptor's schema introspection: JDBC-type-to-AttributeType mapping, default-port resolution, unknown-type rejection, SQLException wrapping, and the base establishConn default. Driven by ScalaMock-backed JDBC interfaces (no live database).
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds focused unit tests in common/workflow-operator to cover SQLSourceOpDesc’s JDBC schema-introspection path (sourceSchema() → querySchema) without requiring a live database, using ScalaMock-backed Connection/DatabaseMetaData/ResultSet mocks.
Changes:
- Introduces a test-only concrete
SQLSourceOpDescsubclass that supplies a mocked JDBC connection and a deterministicupdatePort()implementation. - Adds assertions covering: null-guard behavior, JDBC
Types→AttributeTypemappings, multi-column iteration, default-port resolution, unknown-type failure, andSQLExceptionwrapping. - Adds a regression-style test documenting the base
establishConndefault (null) leading to aNullPointerExceptionwhen schema introspection is attempted.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6078 +/- ##
============================================
+ Coverage 57.21% 57.26% +0.04%
- Complexity 3103 3114 +11
============================================
Files 1130 1130
Lines 43825 43825
Branches 4747 4747
============================================
+ Hits 25075 25096 +21
+ Misses 17313 17289 -24
- Partials 1437 1440 +3
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 365 | 0.223 | 25,953/41,632/41,632 us | 🔴 +23.8% / 🔴 +172.4% |
| ⚪ | bs=100 sw=10 sl=64 | 797 | 0.486 | 123,031/150,066/150,066 us | ⚪ within ±5% / 🔴 +38.5% |
| ⚪ | bs=1000 sw=10 sl=64 | 916 | 0.559 | 1,089,776/1,145,717/1,145,717 us | ⚪ within ±5% / 🔴 +10.5% |
Baseline details
Latest main a53d95a from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 365 tuples/sec | 394 tuples/sec | 770.95 tuples/sec | -7.4% | -52.7% |
| bs=10 sw=10 sl=64 | MB/s | 0.223 MB/s | 0.24 MB/s | 0.471 MB/s | -7.1% | -52.6% |
| bs=10 sw=10 sl=64 | p50 | 25,953 us | 24,333 us | 12,775 us | +6.7% | +103.2% |
| bs=10 sw=10 sl=64 | p95 | 41,632 us | 33,636 us | 15,286 us | +23.8% | +172.4% |
| bs=10 sw=10 sl=64 | p99 | 41,632 us | 33,636 us | 18,795 us | +23.8% | +121.5% |
| bs=100 sw=10 sl=64 | throughput | 797 tuples/sec | 785 tuples/sec | 976.93 tuples/sec | +1.5% | -18.4% |
| bs=100 sw=10 sl=64 | MB/s | 0.486 MB/s | 0.479 MB/s | 0.596 MB/s | +1.5% | -18.5% |
| bs=100 sw=10 sl=64 | p50 | 123,031 us | 125,078 us | 102,557 us | -1.6% | +20.0% |
| bs=100 sw=10 sl=64 | p95 | 150,066 us | 153,610 us | 108,383 us | -2.3% | +38.5% |
| bs=100 sw=10 sl=64 | p99 | 150,066 us | 153,610 us | 115,249 us | -2.3% | +30.2% |
| bs=1000 sw=10 sl=64 | throughput | 916 tuples/sec | 924 tuples/sec | 1,009 tuples/sec | -0.9% | -9.2% |
| bs=1000 sw=10 sl=64 | MB/s | 0.559 MB/s | 0.564 MB/s | 0.616 MB/s | -0.9% | -9.2% |
| bs=1000 sw=10 sl=64 | p50 | 1,089,776 us | 1,081,993 us | 997,695 us | +0.7% | +9.2% |
| bs=1000 sw=10 sl=64 | p95 | 1,145,717 us | 1,130,730 us | 1,036,731 us | +1.3% | +10.5% |
| bs=1000 sw=10 sl=64 | p99 | 1,145,717 us | 1,130,730 us | 1,069,334 us | +1.3% | +7.1% |
Raw CSV
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,548.60,200,128000,365,0.223,25953.38,41631.64,41631.64
1,100,10,64,20,2509.56,2000,1280000,797,0.486,123031.03,150066.44,150066.44
2,1000,10,64,20,21829.83,20000,12800000,916,0.559,1089775.88,1145717.15,1145717.15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this PR?
Add unit test coverage for
SQLSourceOpDesc.querySchema— the abstract SQL source descriptor's JDBC schema-introspection path — selected from the Codecov report. No production-code changes.SQLSourceOpDesc(40.5% covered) is abstract; its concrete subclasses (MySQL/PostgreSQL/AsterixDB) never exercisequerySchemabecause their specs supply no connection. The 27 missed lines are covered here via a small test subclass whoseestablishConnreturns ScalaMock-backedjava.sql.Connection/DatabaseMetaData/ResultSet— no live database.Covered:
nulluntil all fields set)Types→AttributeTypemapping arm (INTEGER/SMALLINT, DOUBLE/NUMERIC, BOOLEAN, BINARY, VARCHAR, BIGINT, TIMESTAMP)updatePort()and multi-column iterationRuntimeExceptionSQLException→ wrapped-RuntimeExceptioncatchestablishConndefault (returns no connection →NullPointerException)Any related issues, documentation, discussions?
Follow-up to the review feedback on #6043: prioritize tests that fill uncovered code paths.
How was this PR tested?
sbt "WorkflowOperator/testOnly *SQLSourceOpDescSpec"— 16 tests, all greensbt "WorkflowOperator/Test/scalafmtCheck"andsbt "WorkflowOperator/scalafixAll --check"— cleanWas this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])