test(amber): add no-DB specs for the dashboard search query tier - #7116
Open
aglinxinyuan wants to merge 1 commit into
Open
test(amber): add no-DB specs for the dashboard search query tier#7116aglinxinyuan wants to merge 1 commit into
aglinxinyuan wants to merge 1 commit into
Conversation
FulltextSearchQueryUtilsSpec was the only spec in the dashboard package. It also proves the approach: a dialect-only `DSL.using(SQLDialect.POSTGRES)` context renders jOOQ Conditions with no connection at all. These three specs extend that to the parts of the tier that are genuinely connection-free. - DashboardResourceSpec: getOrderFields is public and pure. Existing coverage (incidental, via WorkflowResourceSpec) only reached NameAsc/Desc, ExecutionTimeAsc/Desc and EditTime-via-default; this adds CreateTime both ways and the non-matching-orderBy fall-through. It also pins the asc/desc asymmetry -- Desc emits `desc nulls last` while Asc emits a bare `asc` -- which is what makes the behaviour symmetric, since Postgres defaults to NULLS LAST for ASC and NULLS FIRST for DESC. Plus the unknown-resourceType IllegalArgumentException, which is raised before any query is built and so needs no database. - WorkflowSearchQueryBuilderSpec: toEntryImpl and mappedResourceSchema are callable because this object widens the trait's `protected` to public. Covers the projects-aggregate lookup (which rides on jOOQ structural Field equality), both branches of the comma-split, the NULL-privilege fallback to NONE, and the ownership flag both ways. - UnifiedResourceSchemaSpec: the 24-slot projection and the keep-first de-duplication behind allFields, whose counts were measured rather than assumed (24 aliases, collapsing to 6 for the all-defaults schema and 17 for the workflow schema). Assertion strength was checked by mutation: reverting `nulls last`, swapping an order field, inverting the isOwner comparison, changing the privilege fallback, and disabling the de-dup guard each turn tests red. No production file is touched.
Contributor
Automated Reviewer SuggestionsBased on the
|
Contributor
|
| config | throughput | MB/s | latency | max Δ latest / 7d | |
|---|---|---|---|---|---|
| 🔴 | bs=10 sw=10 sl=64 | 429 | 0.262 | 22,629/31,749/31,749 us | 🔴 +12.6% / 🔴 +101.3% |
| 🟢 | bs=100 sw=10 sl=64 | 940 | 0.574 | 108,416/117,542/117,542 us | 🟢 -14.2% / 🔴 +9.5% |
| ⚪ | bs=1000 sw=10 sl=64 | 1,111 | 0.678 | 907,235/929,716/929,716 us | ⚪ within ±5% / 🟢 -11.8% |
Baseline details
Latest main 301e3a8 from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 429 tuples/sec | 477 tuples/sec | 786.12 tuples/sec | -10.1% | -45.4% |
| bs=10 sw=10 sl=64 | MB/s | 0.262 MB/s | 0.291 MB/s | 0.48 MB/s | -10.0% | -45.4% |
| bs=10 sw=10 sl=64 | p50 | 22,629 us | 20,100 us | 12,305 us | +12.6% | +83.9% |
| bs=10 sw=10 sl=64 | p95 | 31,749 us | 30,120 us | 15,774 us | +5.4% | +101.3% |
| bs=10 sw=10 sl=64 | p99 | 31,749 us | 30,120 us | 18,978 us | +5.4% | +67.3% |
| bs=100 sw=10 sl=64 | throughput | 940 tuples/sec | 955 tuples/sec | 999.71 tuples/sec | -1.6% | -6.0% |
| bs=100 sw=10 sl=64 | MB/s | 0.574 MB/s | 0.583 MB/s | 0.61 MB/s | -1.5% | -5.9% |
| bs=100 sw=10 sl=64 | p50 | 108,416 us | 105,377 us | 100,616 us | +2.9% | +7.8% |
| bs=100 sw=10 sl=64 | p95 | 117,542 us | 136,953 us | 107,356 us | -14.2% | +9.5% |
| bs=100 sw=10 sl=64 | p99 | 117,542 us | 136,953 us | 113,255 us | -14.2% | +3.8% |
| bs=1000 sw=10 sl=64 | throughput | 1,111 tuples/sec | 1,128 tuples/sec | 1,031 tuples/sec | -1.5% | +7.7% |
| bs=1000 sw=10 sl=64 | MB/s | 0.678 MB/s | 0.688 MB/s | 0.63 MB/s | -1.5% | +7.7% |
| bs=1000 sw=10 sl=64 | p50 | 907,235 us | 886,818 us | 980,328 us | +2.3% | -7.5% |
| bs=1000 sw=10 sl=64 | p95 | 929,716 us | 925,214 us | 1,027,528 us | +0.5% | -9.5% |
| bs=1000 sw=10 sl=64 | p99 | 929,716 us | 925,214 us | 1,054,298 us | +0.5% | -11.8% |
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,465.81,200,128000,429,0.262,22628.55,31749.12,31749.12
1,100,10,64,20,2127.50,2000,1280000,940,0.574,108415.59,117542.27,117542.27
2,1000,10,64,20,17999.89,20000,12800000,1111,0.678,907234.72,929715.67,929715.67
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7116 +/- ##
============================================
+ Coverage 79.40% 79.59% +0.18%
- Complexity 3803 3833 +30
============================================
Files 1159 1159
Lines 46142 46145 +3
Branches 5128 5127 -1
============================================
+ Hits 36639 36728 +89
+ Misses 7887 7786 -101
- Partials 1616 1631 +15
*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:
|
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
2 participants
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?
FulltextSearchQueryUtilsSpecwas the only spec in the dashboard package. It also proves the approach: a dialect-onlyDSL.using(SQLDialect.POSTGRES)context renders jOOQ parts with no connection, andctx.newRecord(...)builds records purely in memory. These three specs extend that to the rest of the connection-free tier.DashboardResourceSpecgetOrderFields, unknown-resourceTypeguardWorkflowSearchQueryBuilderSpectoEntryImpl,mappedResourceSchemaprotected, widening access to publicUnifiedResourceSchemaSpecapply, thetranslatedFieldSetde-dupFields, allDSL.inline/cast/castNullgetOrderFields— existing coverage was only incidental (viaWorkflowResourceSpec) and reachedNameAsc/Desc,ExecutionTimeAsc/Desc, andEditTimethrough the default. This addsCreateTimeboth ways and the non-matching fall-through. It also pins the asc/desc asymmetry:That asymmetry is the point: Postgres defaults to NULLS LAST for ASC and NULLS FIRST for DESC, so the explicit
nullsLast()on the descending branch is what makes the observable ordering symmetric. Dropping it looks like a harmless simplification and silently moves NULLs to the top of every descending page.toEntryImpl— covers the projects-aggregate lookup, both comma-split branches, the NULL-privilege fallback toNONE, and the ownership flag both ways. The first test deliberately pins the aggregate lookup itself:toEntryImplbuilds a freshgroupConcatDistinct(WORKFLOW_OF_PROJECT.PID)rather than reusing the one inmappedResourceSchema, so the whole projects feature rides on jOOQ comparing two structurally identical aggregates as equal.UnifiedResourceSchema— the counts were measured, not assumed: 24 aliases, collapsing to 6 entries for the all-defaults schema and 17 for the workflow schema.Assertion strength was checked by mutating production and confirming red, then reverting (
git diff -- amber/src/mainis empty):desc().nullsLast()→desc()EditTime→resourceCreationTimeFieldinto(WORKFLOW_OF_USER).getUid == uid→into(USER).getUid != uidNONE→READif (true)Out of scope, and noted in-file:
translateRecord,resultsOwnersInfo,searchAllPublicResourceCall,constructQueryand every fetch path go throughSqlServer.getInstance().createDSLContext().DatasetSearchQueryBuilderis untouched — all five of its members areoverride protected, and Scalaprotectedgrants no same-package access.Two things deliberately not asserted, with the reasoning left in the files:
case None => List()ingetOrderFieldsandcase _ => nullin the privategetColumnFieldare unreachable (the regex yields only four column names, all mapping non-null); and the projects parser'sNumberFormatExceptionon a padded separator is real but unproducible in production, so pinning it would only punish someone hardening the parser.None of these specs reads or writes
FulltextSearchQueryUtils.usePgroonga, so they neither depend on it nor mutate it — worth stating because two existing specs set that JVM-global and never restore it while amber's suites run concurrently.Fragilities found in passing, reported rather than changed:
DashboardWorkflow.ownerIdis read asrecord.into(USER).getUidalthough the projection never selectsUSER.UID— it works only because jOOQ matches the unqualified nameuidagainstworkflow_of_user.uid; andsearchAllResources's per-recordresourceTypematch has no default arm, so changing the inline'workflow'literal becomes a runtimeMatchError.Any related issues, documentation, discussions?
Closes #7115
How was this PR tested?
Three new specs, 27 tests. Run together with the existing sibling spec to confirm nothing in the package regressed — 41 tests, Java 17:
Verified with no Postgres provisioned locally, which is the point.
Test/scalafmtCheckandTest/scalafix --checkboth[success].One brittleness to be aware of: the expected strings (
desc nulls last,cast(null as timestamp),'workflow' as "resourceType") and the de-dup counts are jOOQ 3.19.36 rendering artifacts, so a jOOQ bump could require updating them together.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)