Conversation
The fixture list was a single array, so the only way to benchmark a new vertical was to append to WEB_ACCESS_ALL_TESTS -- which silently changes the composition of the published suite and makes a new run non-comparable with earlier reported numbers. Add WEB_ACCESS_SUITES, a registry of selectable target suites, and a --suite flag that chooses which one a run draws from (--tests still filters within it). The default suite is unchanged, so omitting --suite reproduces exactly the previous behaviour. The first opt-in suite is `automotive`: eight European car classifieds and salvage-auction sources (autoplius.lt, auto24.ee, otomoto.pl, mobile.de, autoscout24.com, iaai.com, copart.com, bidfax.info). Each fixture's URL and containsText were verified against a live fetch, and the antibot labels come from observed response headers rather than assumption -- autoscout24 carries no label because none was observed on that path. Part of S-139626
…uite registry Selecting a suite that does not exist threw, and main().catch prints e.stack, so a typo in --suite produced a stack trace where --providers with no keys and --tests with no matches both produce a single line and exit 1. Resolve the suite in main() next to those two guards; selectTests is left doing nothing but filtering. Add tests for the invariants the suite registry introduces. The one that matters is containsText: validateResponse passes on a 2xx plus a substring match, so a fixture that omits containsText scores a success against an anti-bot block page served with a 200, silently inflating that provider's rate. The test glob widens to dist/*.test.js so a second test file is actually run. Part of S-139626
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeb9b61a4e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const suite = WEB_ACCESS_SUITES[suiteName]; | ||
| if (!suite) { |
There was a problem hiding this comment.
When --suite is an inherited Object.prototype name such as constructor, toString, or __proto__, this lookup returns a truthy non-suite value, so the unknown-suite guard is bypassed. Depending on the name and whether --tests is supplied, the CLI then reports misleading test counts, throws from .filter, or can run an empty benchmark; verify ownership with Object.hasOwn(WEB_ACCESS_SUITES, suiteName) or use a prototype-free registry before accepting the value.
Useful? React with 👍 / 👎.
| export const WEB_ACCESS_SUITES: Record<string, WebAccessTestConfig[]> = { | ||
| default: WEB_ACCESS_ALL_TESTS, | ||
| automotive: WEB_ACCESS_AUTOMOTIVE_TESTS, |
There was a problem hiding this comment.
When consumers use the documented programmatic package API, both this registry and WEB_ACCESS_AUTOMOTIVE_TESTS are unreachable: package.json exposes only the package root, and src/index.ts still re-exports only WEB_ACCESS_ALL_TESTS and WEB_ACCESS_BENCHMARK_CONFIG. Consequently the new automotive suite cannot be passed to runWebAccessBenchmarkSuite through the supported package import, while deep-importing tests.const is blocked by the package exports map; add the new suite exports to src/index.ts.
Useful? React with 👍 / 👎.
2 participants
Why
Every run draws from the same fixed target list. That is what keeps the published numbers comparable across runs, and it is also why there is no way to ask how providers do on the sources in your own vertical. Today you either edit
WEB_ACCESS_ALL_TESTS, which changes the published composition so the next run cannot be compared against the posted results, or you maintain a fork.Summary
WEB_ACCESS_SUITES, a registry of selectable target sets, plus a--suite <name>flag.defaultisWEB_ACCESS_ALL_TESTSuntouched, so an existing command draws an identical target list.automotivesuite of 8 car classifieds and salvage-auction sources: autoplius.lt, auto24.ee, otomoto.pl, mobile.de, autoscout24, iaai.com, copart.com, bidfax.info. EverycontainsTextwas confirmed against a live fetch of the listed URL rather than assumed, and eachantibotlabel comes from observed response headers. autoscout24 carries no label because none appeared on that path.--testsnow filters within the selected suite rather than the global list.--suiteprints one line and exits 1, the way the CLI already reports no active providers and no matching tests. It previously threw, and the top-level handler prints the stack.containsText:validateResponsepasses on a 2xx plus a substring match, so a fixture that omits it would score a block page served with a 200 as a success and inflate that provider's rate.Considered and rejected: adding these targets straight to
WEB_ACCESS_ALL_TESTS. That needs no new flag, but it changes the published suite's composition, so the next run could not be compared against the posted results.Test plan
npm run typecheckcleannpm testpasses 8 / fails 0 (5 existing, 3 new)--suite automotiveprintsSuite: automotive | tests: 8Suite: default | tests: 100, the same target list as before this change--suite nopeprintsUnknown suite "nope" — choose one of: default, automotiveand exits 1 with no stack tracecontainsTextconfirmed present in a live response body for its URL