test(frontend, auth): pin the email validator, re-home the unauthorized spec - #7919
Merged
aglinxinyuan merged 1 commit intoAug 26, 2026
Merged
Conversation
…ed spec email.ts had no spec at all despite sitting on the sign-in path, and its coverage came entirely from indirect drivers -- so the regex could be replaced with /@/, lose its trailing anchor, or be narrowed to \.com$ and nothing in the repository would fail. New spec, 7 tests, file to 100%. UnauthorizedExceptionMapperSpec tested a common/auth class from inside access-control-service. JaCoCo reports per module, so it credited the file with nothing; moved to common/auth/src/test, which takes it to 100%. HONEST YIELD: +5 fully-covered lines and +1 arm, of which +3 lines are re-attribution of already-running behaviour rather than new testing, and one frontend line is promoted only by an unreachable null-guard arm. Genuinely new coverage is 2 lines. What earns the PR is the five mutants that survived the first draft, three of them gutting the email regex. 16 mutations, 15 killed. The survivor -- flipping enableSuppression -- is recorded rather than claimed equivalent: it is observable via getSuppressed in principle, but nothing in the repo calls it. LargeBinaryManager was in scope and is dropped at +0; reaching it needs a build.sbt change with LICENSE-binary fallout.
Contributor
Automated Reviewer SuggestionsBased on the
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7919 +/- ##
============================================
+ Coverage 92.47% 92.52% +0.04%
- Complexity 4588 4606 +18
============================================
Files 1173 1173
Lines 47347 47355 +8
Branches 5306 5302 -4
============================================
+ Hits 43784 43815 +31
+ Misses 1979 1972 -7
+ Partials 1584 1568 -16
*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 | 387 | 0.236 | 25,534/32,443/32,443 us | 🟢 -10.5% / 🔴 +126.3% |
| 🔴 | bs=100 sw=10 sl=64 | 784 | 0.478 | 125,447/174,595/174,595 us | 🔴 +5.6% / 🔴 +75.5% |
| ⚪ | bs=1000 sw=10 sl=64 | 928 | 0.566 | 1,079,106/1,136,077/1,136,077 us | ⚪ within ±5% / 🔴 +18.5% |
Baseline details
Latest main 84e2d7d from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---|---|---|---|---|
| bs=10 sw=10 sl=64 | throughput | 387 tuples/sec | 400 tuples/sec | 833.1 tuples/sec | -3.3% | -53.5% |
| bs=10 sw=10 sl=64 | MB/s | 0.236 MB/s | 0.244 MB/s | 0.508 MB/s | -3.3% | -53.6% |
| bs=10 sw=10 sl=64 | p50 | 25,534 us | 23,522 us | 11,860 us | +8.6% | +115.3% |
| bs=10 sw=10 sl=64 | p95 | 32,443 us | 36,251 us | 14,334 us | -10.5% | +126.3% |
| bs=10 sw=10 sl=64 | p99 | 32,443 us | 36,251 us | 18,194 us | -10.5% | +78.3% |
| bs=100 sw=10 sl=64 | throughput | 784 tuples/sec | 812 tuples/sec | 1,085 tuples/sec | -3.4% | -27.7% |
| bs=100 sw=10 sl=64 | MB/s | 0.478 MB/s | 0.496 MB/s | 0.662 MB/s | -3.6% | -27.8% |
| bs=100 sw=10 sl=64 | p50 | 125,447 us | 118,752 us | 92,973 us | +5.6% | +34.9% |
| bs=100 sw=10 sl=64 | p95 | 174,595 us | 177,596 us | 99,460 us | -1.7% | +75.5% |
| bs=100 sw=10 sl=64 | p99 | 174,595 us | 177,596 us | 107,737 us | -1.7% | +62.1% |
| bs=1000 sw=10 sl=64 | throughput | 928 tuples/sec | 923 tuples/sec | 1,111 tuples/sec | +0.5% | -16.5% |
| bs=1000 sw=10 sl=64 | MB/s | 0.566 MB/s | 0.563 MB/s | 0.678 MB/s | +0.5% | -16.6% |
| bs=1000 sw=10 sl=64 | p50 | 1,079,106 us | 1,088,724 us | 915,816 us | -0.9% | +17.8% |
| bs=1000 sw=10 sl=64 | p95 | 1,136,077 us | 1,146,027 us | 959,066 us | -0.9% | +18.5% |
| bs=1000 sw=10 sl=64 | p99 | 1,136,077 us | 1,146,027 us | 993,644 us | -0.9% | +14.3% |
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,517.32,200,128000,387,0.236,25534.11,32442.61,32442.61
1,100,10,64,20,2552.26,2000,1280000,784,0.478,125446.90,174594.85,174594.85
2,1000,10,64,20,21562.23,20000,12800000,928,0.566,1079105.66,1136076.77,1136076.77
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?
Two small, unrelated things that share one property: the behaviour is exercised today but asserted nowhere.
1.
frontend/src/app/common/util/email.tshad no spec. It is the only un-spec'd file in itsutildirectory, and it is on the sign-in path —AuthServicereadsemailout of the email-request modal'sgetValues(). Newemail.spec.ts, 7 tests, taking the file to 100% lines and branches.2.
UnauthorizedExceptionMapperSpecwas in the wrong module. It testscommon/auth'sUnauthorizedException, but lived inaccess-control-service. JaCoCo reports per module, so it credited that file with nothing. Moved tocommon/auth/src/test, which takesUnauthorizedException.scalafrom 66.7% to 100%.email.tsUnauthorizedException.scalaBoth baselines reproduce Codecov's published figures digit-for-digit, measured whole-module with no filter on either side.
The honest yield is smaller than those percentages suggest
+5 fully-covered lines and +1 branch arm — of which +3 lines are re-attribution, not new testing. The Scala half moves already-tested, already-CI-running behaviour into the module where it counts; it adds one new assertion and nothing else. And on the frontend half, one of the three lines is promoted only by an unreachable null-guard arm, so that half is +2 lines and +1 arm rather than +3 and +2.
So the genuinely new coverage here is 2 lines. I would rather lead with that than with two 100%s.
What actually earns the PR
The email validator could be gutted and nothing in the repository would notice. Five mutants survived the first draft, three of them on
email.ts:/@/\.com$.comTLDUnauthorizedException's reason-phrase overloadUnauthorizedException's status constantThat is the point of the frontend spec: the file's coverage was largely duplicated by indirect drivers, but its behaviour was duplicated nowhere. The indirect drivers are
auth.service.spec.tsandtexera-login.component.spec.ts— the latter reaching it through a static call, which the first draft got wrong when it inventoried them.Verification
16 mutations, 15 killed, 1 survivor.
The survivor, stated plainly and not dressed up as equivalent: flipping
UnauthorizedException.scala:43'senableSuppression = falsetotrueleaves the whole 108-test module green. It is observable in principle throughgetSuppressed, but nothing in the repository calls it, so there is no honest assertion to make. Recorded rather than papered over.Deliberately not included
LargeBinaryManagerwas the third file in scope and is dropped at +0 lines. Reaching it needs abuild.sbtchange with LICENSE-binary fallout, which the test-only constraint forbids.Three tests added in the repair pass move zero counters — the lcov is byte-identical with and without them — and they are kept only for the mutants they kill.
No production file is touched.
access-control-servicewas re-compiled after the move to confirm nothing there depended on the relocated spec.Any related issues, documentation, discussions?
Closes #7918
How was this PR tested?
Auth/Test/scalafmtCheckpasses,AccessControlService/Test/compilesucceeds after the move, andyarn format:cipasses. The new frontend spec carries the Apache licence header.Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)