From c8aaa984dd3a94772f9beac523d3be4500653505 Mon Sep 17 00:00:00 2001 From: "pip-robot[bot]" Date: Sun, 30 Aug 2026 22:58:21 +0000 Subject: [PATCH] test: guard the pokersolver behaviours bestFive slices on bestFive() does slice(0, 5) and that line is only correct because the solver returns the cards that make the hand first, in its own order. That was asserted in a doc comment. Three of the shapes were already covered; two were not. - A full house whose trips rank below its pair (3s 3h 3d As Ah). This is the one case where "first five" and "five highest" disagree, so it is the case that pins the slice. - The royal-flush name/descr split. handPhrase() reads descr because a royal is *named* "Straight Flush"; if descr ever changed, the recap and every drill grade would call a royal a straight flush and no name assertion would fail. Also covers every category name handPhrase maps, so a rename shows up as a failing test rather than a dropped clause. - One direct assertion on the raw solved.cards shape (six-card flush, two trips, six-card straight flush returning five, the low ace as '1'), because anything reading solved.cards inherits those traps and bestFive hides them. Two fixes to the same trap while here: pokersolver.d.ts said cards was always five, which is what leads someone to read it raw; and bestFiveOf in learnExamples.test.ts did read it raw, through an unknown cast, and was safe only because no fixture is a six-card flush. It now goes through bestFive. Verified against the installed pokersolver@2.1.4, which is pinned at ^2.1.4 and last published July 2020. Refs playpip/technology#78. --- src/types/pokersolver.d.ts | 7 ++-- tests/handEval.test.ts | 66 ++++++++++++++++++++++++++++++++++++- tests/learnExamples.test.ts | 13 +++++--- 3 files changed, 79 insertions(+), 7 deletions(-) diff --git a/src/types/pokersolver.d.ts b/src/types/pokersolver.d.ts index 708c681..98d6b11 100644 --- a/src/types/pokersolver.d.ts +++ b/src/types/pokersolver.d.ts @@ -23,8 +23,11 @@ declare module 'pokersolver' { /** Numeric category rank (higher = stronger hand category). */ rank: number /** - * The five cards the solver used, its own order: the cards that make the - * hand first, then the kickers, each descending. + * The cards the solver used, its own order: the cards that make the hand + * first, then the kickers, each descending. **Not always five** - where + * more than five are eligible (a six-card flush, a full house made of two + * trips) every eligible card is here. The first five are the hand; use + * `bestFive()` in `lib/poker/handEval` rather than reading this directly. */ cards: SolvedCard[] /** Solve the best 5-card hand from 5–7 card strings like ["Ah","Kd",...]. */ diff --git a/tests/handEval.test.ts b/tests/handEval.test.ts index ba41334..2983469 100644 --- a/tests/handEval.test.ts +++ b/tests/handEval.test.ts @@ -1,5 +1,5 @@ import test from 'ava' -import { bestFive, evaluateHand, determineWinners } from '@/lib/poker/handEval' +import { bestFive, evaluateHand, determineWinners, handPhrase } from '@/lib/poker/handEval' import { cardToString, cardFromString, type Card } from '@/lib/poker/cards' const h = (...s: string[]): Card[] => s.map(cardFromString) @@ -53,12 +53,76 @@ test('bestFive is five cards, in the solver’s order, even where more are eligi t.is(five(h('Ah', 'Kd'), h('As', '7d', '2c', 'Th', '4s')), 'Ah As Kd Th 7d') }) +// The case that makes the slice legal rather than lucky. A full house whose +// trips rank *below* its pair is the one shape where "first five" and "five +// highest" disagree: sorted by rank the aces would lead and the reveal would +// show a pair of aces and a pair of threes. The solver leads with the trips. +test('bestFive keeps the solver’s order when the trips rank below the pair', (t) => { + t.is(five(h('3s', '3h'), h('3d', 'As', 'Ah', '2c', '4d')), '3s 3h 3d As Ah') +}) + // The ace the solver renames to '1' when it plays low. Left as '1' it would be // an unreadable card face and a rank our own type does not have. test('bestFive maps a low ace back to an ace', (t) => { t.is(five(h('Ac', 'Kh'), h('5h', '4d', '3h', '2d', '9s')), '5h 4d 3h 2d Ac') }) +// bestFive slices, so the overflow itself never reaches a caller through it. +// Anything else reading solved.cards inherits it, and the wrapper's whole +// claim is that the first five are the hand, so assert the raw shape once here +// rather than leaving it as a sentence in a doc comment. +test('the solver overflows past five only where more than five are eligible', (t) => { + const raw = (hole: Card[], board: Card[]): string => + evaluateHand(hole, board) + .solved.cards.map((c) => c.value + c.suit) + .join(' ') + + // Six-card flush and two trips: six cards back, hand first. + t.is(raw(h('9d', '7d'), h('Ad', 'Td', '5h', 'Kd', '4d')), 'Ad Kd Td 9d 7d 4d') + t.is(raw(h('Ks', 'Kd'), h('Ah', 'Ad', 'As', 'Kh', '2c')), 'Ah Ad As Ks Kd Kh') + // A six-card straight flush does not overflow: a straight is five or nothing, + // which is why the doc comment names flushes and full houses and not straights. + t.is(raw(h('9s', '8s'), h('7s', '6s', '5s', '4s', '2d')), '9s 8s 7s 6s 5s') + // And the low ace arrives as '1', a rank our Card type does not have. + t.is(raw(h('Ac', 'Kh'), h('5h', '4d', '3h', '2d', '9s')), '5h 4d 3h 2d 1c') +}) + +// handPhrase leans on a quirk: a royal flush is *named* "Straight Flush" and +// only the description tells them apart. If descr ever stopped being exactly +// "Royal Flush" the recap and every drill grade would quietly call one a +// straight flush, and no name assertion anywhere would fail. +test('handPhrase tells a royal flush from a straight flush by description alone', (t) => { + const royal = evaluateHand(h('As', 'Ks'), h('Qs', 'Js', 'Ts', '2c', '3d')) + t.is(royal.name, 'Straight Flush') + t.is(royal.description, 'Royal Flush') + t.is(handPhrase(royal), 'a royal flush') + + const straightFlush = evaluateHand(h('Ks', 'Qs'), h('Js', 'Ts', '9s', '2c', '3d')) + t.is(straightFlush.name, 'Straight Flush') + t.is(handPhrase(straightFlush), 'a straight flush') +}) + +// Every name the solver can return has a phrase, or the caller drops the clause +// rather than shipping "won with undefined". Cheap to state, and it is the +// list that would go stale first if a name were ever renamed. +test('handPhrase covers every category the solver names', (t) => { + const cases: [string, Card[], Card[]][] = [ + ['high card', h('Ah', 'Kd'), h('7s', '5c', '2d', '9h', 'Jc')], + ['a pair', h('Ah', 'Ad'), h('7s', '5c', '2d', '9h', 'Jc')], + ['two pair', h('Ah', 'Ad'), h('Ks', 'Kc', '2d', '9h', 'Jc')], + ['three of a kind', h('Ah', 'Ad'), h('As', '5c', '2d', '9h', 'Jc')], + ['a straight', h('9h', '8d'), h('7s', '6c', '5d', '2h', 'Jc')], + ['a flush', h('Ah', 'Kh'), h('2h', '7h', 'Th', '3c', '4d')], + ['a full house', h('Ah', 'Ad'), h('As', 'Kh', 'Kd', '3c', '4d')], + ['four of a kind', h('Ah', 'Ad'), h('As', 'Ac', 'Kd', '3c', '4d')], + ['a straight flush', h('Ks', 'Qs'), h('Js', 'Ts', '9s', '2c', '3d')], + ['a royal flush', h('As', 'Ks'), h('Qs', 'Js', 'Ts', '2c', '3d')], + ] + for (const [expected, hole, board] of cases) { + t.is(handPhrase(evaluateHand(hole, board)), expected, `${expected}: no phrase`) + } +}) + test('kicker decides when top pair ties', (t) => { const board = h('As', '7d', '2c', 'Th', '4s') const { winners } = determineWinners( diff --git a/tests/learnExamples.test.ts b/tests/learnExamples.test.ts index 4564f0b..f59a7ec 100644 --- a/tests/learnExamples.test.ts +++ b/tests/learnExamples.test.ts @@ -1,10 +1,15 @@ import test from 'ava' import { ACE_RUNS, BEST_FIVE, CAN_YOU_CHECK, WHO_WINS, toCards } from '@/config/learnExamples' -import { determineWinners, evaluateHand, type EvaluatedHand } from '@/lib/poker/handEval' +import { cardToString } from '@/lib/poker/cards' +import { bestFive, determineWinners, evaluateHand, type EvaluatedHand } from '@/lib/poker/handEval' -/** The five cards the evaluator actually used, as "Kh" strings. */ -const bestFiveOf = (solved: EvaluatedHand): string[] => - (solved.solved as unknown as { cards: unknown[] }).cards.map(String) +/** + * The five cards the evaluator actually used, as "Kh" strings. Goes through + * bestFive rather than reading solved.cards: the raw list is six cards on a + * six-card flush or two trips, and holds a low ace as '1'. This used to read + * it raw and was safe only because no fixture had that shape. + */ +const bestFiveOf = (solved: EvaluatedHand): string[] => bestFive(solved).map(cardToString) // The whole point of this file: every worked example on a guide page states who // wins a hand, in public, to people learning the rules. Getting one wrong is