fix(test): stop Yahoo API test panic from aborting the whole suite - #196
Merged
Conversation
TestYahooCheckByAPI used assert.NoError, which does not halt on failure. When check() returns an error, execution continued to res.HostExists with res == nil and panicked, killing the test binary. That test is AfterShip#44 of ~90, so every test after it stopped running -- all of smtp_test.go, suggestion_test.go, util_test.go and verifier_test.go, dead since the Yahoo endpoint broke around 2025-12-09. Use require.NoError so the subtest fails cleanly instead, and pass the subtest's own *testing.T rather than the parent's. The tt parameter was declared but unused (legal for parameters, so it compiled), which meant assertions were attributed to the parent test. It also matters for require: FailNow must be called from the goroutine running the test, and subtests run in their own. Verified locally with the same flags CI uses (go test -race -covermode atomic ./...): before: panic at test AfterShip#44, 47 tests never executed after: suite completes in 27.4s, 2 failures, 88 passes This does not turn CI green. TestYahooCheckByAPI and TestCheckSMTPOK_ByApi still fail because the upstream Yahoo endpoint returns 404 (see AfterShip#195). It does restore CI signal for everything else in the package. Refs AfterShip#195 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
testifylint's bool-compare rule flags assert.Equal(tt, true, x). These lines already violated it before this branch, but golangci-lint runs with only-new-issues: true, so they were never reported until the previous commit touched them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
mzcaptainyang
approved these changes
Aug 24, 2026
hodge1997
approved these changes
Aug 24, 2026
NeoCN
added a commit
that referenced
this pull request
Aug 24, 2026
* feat!: remove non-functional Yahoo API verifier The Yahoo API verifier has been broken since ~2025-12: the endpoint it posted to, /account/module/create?validateField=userId, now returns 404. Its test panicked and, until #196, took the rest of the suite down with it. Investigation of Yahoo's current pages shows the underlying capability is gone, not merely relocated. Three independent flows were checked: - Signup, old per-field validation: endpoint is 404. - Signup, current page: a Next.js App Router single form POST to /account/create with no separate availability check; the only way to probe a username is to submit a full registration. - Forgot-password / find-username: also a single form POST guarded by a browser-fingerprint field, and it asks for a *recovery* email rather than the address under test. There is no public path left to determine whether a Yahoo address exists, so this follows the precedent of #113 (Gmail verifier removal). BREAKING CHANGE: EnableAPIVerifier(YAHOO) is removed. The exported YAHOO constant is gone and EnableAPIVerifier now returns an error for every vendor. The smtpAPIVerifier interface and the apiVerifiers dispatch remain as an extension point for future vendors. Verified with go build/vet, golangci-lint (0 issues), and the full suite (go test -race -covermode atomic): ok, 89.5% coverage, no panics. This is the first clean run since December. Refs #195 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: add changelog entries for the Yahoo verifier removal Also corrects the v1.4.0 entry's link text: it read #76 while pointing at pull/88. #88 is the Gmail/Yahoo API support PR; #76 is DisableCatchAllCheck, already referenced correctly under v1.3.3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Problem
TestYahooCheckByAPIusesassert.NoError, which records a failure but does not halt. Whencheck()returns an error, execution continues tores.HostExistswithres == nilandpanics, which kills the test binary.
That test is #44 of ~90 in the package, so every test after it stops running. Since the
upstream Yahoo endpoint broke around 2025-12-09 (see #195), the following have not executed in
CI for ~8.5 months:
smtp_test.go— every SMTP and catch-all testsuggestion_test.goutil_test.goverifier_test.goEvery
ci.ymlrun since 2025-12-09 has failed, and the panic meant the reported failure toldyou about one test while silently hiding 47 others.
Change
One file, two commits.
require.NoErrorinstead ofassert.NoError, pass the subtest's own*testing.T, and switch the bool comparisons toassert.True/assert.False:requireinstead ofassert— fails the subtest immediately rather than continuing onto dereference a nil
*SMTP.ttinstead oft— the closure parameter was declared but never used (legal forparameters, so it compiled), so assertions were attributed to the parent test. It also
matters for
require:FailNowmust be called from the goroutine running the test, andsubtests run in their own.
assert.True/assert.False—testifylint'sbool-comparerule rejectsassert.Equal(tt, true, x). Those lines already violated it, butonly-new-issues: truemeant it was never surfaced until this branch touched them.
Verification
Run locally with the same flags CI uses,
go test -race -covermode atomic ./...:The two remaining failures share one root cause —
yahoo check by api, no sessionIndex:TestYahooCheckByAPITestCheckSMTPOK_ByApi(smtp_test.go:67, which goes throughEnableAPIVerifier(YAHOO);it never panicked only because it does not dereference the nil result)
Spot-checked that the previously-dead tests genuinely pass rather than merely run: a targeted
run over
verifier_test.go,smtp_test.go,suggestion_test.goandutil_test.gogives 21passes and
ok.Confirmed on CI as well — the run on this branch reaches the Test step, completes in 11.7s
with no panic, and reports those same two failures and nothing else. So the SMTP tests in
smtp_test.godo pass on GitHub's runners; they had simply not been reached since December.Scope
This does not make CI green. The two tests above will keep failing until there is a
decision on the Yahoo verifier itself — #195 lays out the options (remove it following the
Gmail precedent in #113, chase Yahoo's replacement endpoint, or give
catch_alla way toexpress "undetermined"). That decision is deliberately out of scope here, since removing
EnableAPIVerifier(YAHOO)would be a breaking public API change.What this does do is restore CI signal for the other 88 tests, so unrelated PRs stop carrying
a red check that hides whether they actually broke anything. #188 is a current example — the
test it adds lives in
verifier_test.goand has never executed.Refs #195
🤖 Generated with Claude Code