Conversation
Add 61 test files with 365 tests covering scrapers, API routes, lib utilities, and middleware. Excludes React client context providers (currency/i18n/theme) from coverage as they require jsdom.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 31 minutes and 17 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThe PR adds comprehensive Vitest unit test coverage spanning API routes, libraries, middleware, scrapers for 40+ countries, and type definitions. A Prisma generation step is added to CI, and three UI component files are excluded from coverage reporting. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The test job was missing prisma generate, causing db.test.ts to fail with ERR_MODULE_NOT_FOUND for @/generated/prisma/client on CI.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (23)
src/scrapers/denmark.test.ts (2)
285-286: Prefer lookup by fuelType over positional indexing.
prices[0].currencydepends on the iteration order of the mapper over thepricesarray. It works today, but a.find(p => p.fuelType === "B7")assertion is sturdier and makes the intent explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/denmark.test.ts` around lines 285 - 286, The assertion relies on array order; instead of indexing prices[0], locate the B7 entry by using a lookup like prices.find(p => p.fuelType === "B7") and assert its currency equals "DKK" (and optionally assert the find result is defined). Update the test in denmark.test.ts to reference the found object rather than positional indexing to make the intent explicit and robust.
359-414: Strengthen the 401-fallback assertion.
callCount > 1only proves something happened after the 401. It would still pass if the retry went to an unrelated endpoint or failed silently. Assert that both the DrivstoffAppen auth and stations endpoints were actually hit (similar to lines 278-279) to lock in the intended fallback behavior.♻️ Proposed tightening
- let callCount = 0; + const calls: string[] = []; vi.mocked(fetch).mockImplementation(async (input: string | URL | Request) => { const url = typeof input === "string" ? input : input.toString(); - callCount++; + calls.push(url); @@ const { stations } = await scraper.fetch(); expect(stations).toHaveLength(1); expect(stations[0].externalId).toBe("dk-da-7001"); - expect(callCount).toBeGreaterThan(1); // Went past the first call + expect(calls.some((u) => u.includes("fuelprices.dk"))).toBe(true); + expect(calls.some((u) => u.includes("authorization-sessions"))).toBe(true); + expect(calls.some((u) => u.includes("/stations"))).toBe(true);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/denmark.test.ts` around lines 359 - 414, The test currently only asserts callCount > 1 which is weak; update the "falls back to DrivstoffAppen on 401 from Fuelprices.dk" test (using DenmarkScraper) to explicitly assert that the DrivstoffAppen auth endpoint ("authorization-sessions") and the DrivstoffAppen stations endpoint ("/stations") were invoked; you can inspect vi.mocked(fetch).mock.calls (or capture booleans in the mock implementation) and add two expects verifying at least one fetch call string includes "authorization-sessions" and one includes "/stations" to ensure the fallback path was actually exercised.src/scrapers/serbia.test.ts (3)
255-298: Test title vs. assertion comment drift.The test is named "skips stations with zero coordinates" but the inline comment on Line 296 says filtering happens in
fetchNISStationsviaLatitude !== 0. If the scraper actually rejects any falsy/zero lat or lng, consider adding a second fixture row with onlyLongitude: 0(and a valid latitude) to lock in both branches. Otherwise regressions that only check latitude will slip through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/serbia.test.ts` around lines 255 - 298, The test title claims both coordinates are skipped but the fixture only covers Latitude === 0; update the test for SerbiaScraper/fetch to cover the Longitude === 0 case too by adding a second station object in zeroCoordStation with a non-zero Latitude and Longitude: 0 (or alternatively adjust the assertion/comment to reflect only Latitude is filtered); ensure the mocked nisgazprom response includes both items and the final expect(stations).toHaveLength(0) still holds so regressions that only filter latitude or longitude are caught.
14-17: GlobalsetTimeoutoverride is a blunt instrument.Replacing
setTimeoutglobally forces every timer (including any used by Vitest, Node internals, or libraries loaded during the dynamic import) to fire on the next tick. Prefervi.useFakeTimers()+vi.runAllTimersAsync(), or have the scraper accept an injectable sleep/delay, so only the code under test is affected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/serbia.test.ts` around lines 14 - 17, The test currently overrides globalThis.setTimeout via origSetTimeout which affects all timers; instead switch to Vitest's timer utilities or inject a sleep: replace the global stub with vi.useFakeTimers() in the test setup, call vi.runAllTimersAsync() (or advanceTimersToNext/advanceTimersByTime as appropriate) around the dynamic import or the code that triggers delays, and restore timers with vi.useRealTimers() after; alternatively update the scraper to accept an injectable delay/sleep function and pass a no-wait stub from the test rather than touching globalThis.setTimeout (referencing origSetTimeout, globalThis.setTimeout, vi.useFakeTimers, vi.runAllTimersAsync, and the scraper's sleep parameter).
349-394: Good negative-path coverage, but silent HTTP 500s deserve a log assertion.The test confirms the scraper doesn't throw on upstream 500s from
cenagoriva.rs, which is the right resilience behavior — but swallowing those errors with zero observability is a production-monitoring hazard. Consider spying onconsole.warn/console.error(or whichever logger the scraper uses) and asserting it was called, so a future "silently eat all errors" refactor can't regress this silently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/serbia.test.ts` around lines 349 - 394, Update the "handles cenagoriva.rs page errors gracefully" test to assert that the scraper logs the upstream 500 error: spy on the logger used by SerbiaScraper (e.g., console.warn or console.error) before calling scraper.fetch(), exercise the mocked fetch that returns the 500 for cenagoriva.rs, then assert the spy was called (and optionally that the message contains "cenagoriva" or "500"); finally restore the spy/mock after the assertion so subsequent tests are unaffected. Ensure you reference the SerbiaScraper.fetch behavior and the cenagoriva.rs fetch case when adding the assertion.src/scrapers/sweden.test.ts (2)
155-155:url.includes("/stations")is load-bearing and fragile.The auth URL is matched first so ordering saves you today, but any future auth endpoint containing
/stations(or a refactor reordering the branches) will silently route to the wrong mock. Consider anchoring to a more specific fragment (e.g.countryId=2or a pathname check) for consistency with the first test.Also applies to: 200-200, 261-261, 306-306, 353-353
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/sweden.test.ts` at line 155, The test's URL matching using url.includes("/stations") is fragile and can misroute requests; update the mock predicate(s) (the conditional(s) that currently use url.includes("/stations")) to match a more specific fragment such as checking the request pathname equals "/stations" or matching the query param "countryId=2" (or another exact identifier used in the test) so it won't collide with auth endpoints; apply the same change to the other similar occurrences in this test file (the other url.includes("/stations") conditions) to ensure deterministic routing.
272-272: Substring matching in error assertions is fragile. Tests at lines 272 and 289 pass because "HTTP 503" and "auth failed" are substrings of the actual error messages (DrivstoffAppen stations API returned HTTP 503: ...andDrivstoffAppen auth failed: HTTP ${res.status}). However, any rewording of these error messages will break these tests silently or cause unexpected failures. Consider asserting against the full error message or using a regex pattern instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/sweden.test.ts` at line 272, The tests use fragile substring assertions like await expect(scraper.fetch()).rejects.toThrow("HTTP 503") and should instead assert a stable pattern or the full message; update the failing assertions in src/scrapers/sweden.test.ts (the expect(...).rejects.toThrow calls around scraper.fetch()) to either compare against the full error string thrown by the code (e.g., the complete "DrivstoffAppen stations API returned HTTP 503: ..." message) or use a precise regex with toThrow(/DrivstoffAppen stations API returned HTTP 503/) and likewise replace the "auth failed" substring check with a full-message or regex matching "DrivstoffAppen auth failed: HTTP \\d+" so test intent is robust to wording changes.src/scrapers/luxembourg.test.ts (1)
11-14: Consider addingvi.unstubAllGlobals()for symmetry.
vi.restoreAllMocks()does not unstub globals set viavi.stubGlobal. It happens to work here becausebeforeEachre-stubsfetch, but sibling test files in this PR (e.g.,australia.test.ts) do callvi.unstubAllGlobals(). Aligning the teardown avoids cross-file leakage if tests ever run interleaved or if more globals are added later.🔧 Suggested tweak
afterEach(() => { vi.restoreAllMocks(); vi.resetModules(); + vi.unstubAllGlobals(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/luxembourg.test.ts` around lines 11 - 14, The test teardown currently calls afterEach which runs vi.restoreAllMocks() and vi.resetModules(), but it misses vi.unstubAllGlobals() so globals stubbed with vi.stubGlobal may leak; update the afterEach cleanup to also call vi.unstubAllGlobals() (alongside vi.restoreAllMocks() and vi.resetModules()) to mirror other tests like australia.test.ts and ensure globals stubbed in beforeEach are fully removed between tests.src/scrapers/australia.test.ts (2)
60-76: Consider asserting the canonical fuel code.The test validates price value and
AUDcurrency but does not checkcaltexPrice!.fuelType. Adding an assertion (e.g.,expect(caltexPrice!.fuelType).toBe("E10")or whichever code the scraper emits for ULP) would guard against silent regressions in fuel-type mapping and keep parity with the other scraper tests in this PR.As per coding guidelines:
Use EU harmonized fuel type codes (E5, E10, E5_98, B7, B7_PREMIUM, LPG, CNG, H2) as canonical internal IDs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/australia.test.ts` around lines 60 - 76, The test is missing an assertion for the canonical fuel code on the price object; update the Australia scraper test (after finding caltexPrice from prices returned by scraper.fetch()) to assert caltexPrice!.fuelType equals the EU harmonized code your scraper emits for that pump (e.g., "E10" or the correct ULP mapping) so regressions in fuel-type mapping are caught; reference the caltexPrice variable and the prices array when adding the expect assertion and follow the project guideline to use codes like E5, E10, E5_98, B7, B7_PREMIUM, LPG, CNG, or H2.
10-10: Usevi.useFakeTimers()instead of stubbingsetTimeoutglobally.Stubbing
setTimeoutglobally can interfere with Vitest's internal microtask and timer scheduling. The idiomatic approach isvi.useFakeTimers({ shouldAdvanceTime: true }), which automatically advances mocked time based on real system time. This preserves the scraper's real delay semantics while avoiding potential test framework interference.Call
vi.useRealTimers()inafterEachfor proper cleanup:Suggested change
beforeEach(() => { vi.stubGlobal("fetch", vi.fn()); - vi.stubGlobal("setTimeout", (fn: () => void) => { fn(); return 0 as unknown as NodeJS.Timeout; }); + vi.useFakeTimers({ shouldAdvanceTime: true }); }); afterEach(() => { vi.restoreAllMocks(); + vi.useRealTimers(); vi.resetModules(); - vi.unstubAllGlobals(); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/australia.test.ts` at line 10, The test currently stubs setTimeout globally via vi.stubGlobal("setTimeout", ...) which interferes with Vitest internals; replace that stub with vi.useFakeTimers({ shouldAdvanceTime: true }) at the start of the test setup (remove the vi.stubGlobal call) and add vi.useRealTimers() in an afterEach hook to restore real timers; ensure the new fake-timer setup preserves the scraper's delay semantics and that teardown always runs to avoid leaking fake timers.vitest.config.ts (1)
18-18: Coverage exclusions look reasonable.Excluding the three React client providers is pragmatic given the node-only test environment. Consider tracking a follow-up to enable jsdom selectively and restore coverage on those modules, rather than leaving them permanently excluded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vitest.config.ts` at line 18, The coverage exclusions in the vitest config currently exclude the three React client provider modules (the exclude array entry containing "src/lib/currency.tsx", "src/lib/i18n.tsx", "src/lib/theme.tsx"); add a short actionable follow-up: add a TODO comment next to the exclude array and/or create a tracked issue to enable jsdom for selective tests and restore these modules to coverage once a jsdom-based test strategy is implemented, referencing the vitest config exclude array and the three module paths so they can be re-included later.src/scrapers/austria.test.ts (1)
10-10:setTimeoutstub invokes synchronously — be aware of ordering.Calling
fn()synchronously in the stub deviates from real timer semantics (microtask vs macrotask ordering). Works here because awaited Promises still resolve, but if the scraper ever relies on yielding to the event loop between grid calls, this stub could mask an ordering bug. ConsiderqueueMicrotask(fn)for closer-to-real behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/austria.test.ts` at line 10, The current test stub for setTimeout (vi.stubGlobal("setTimeout", (fn: () => void) => { fn(); return 0 as unknown as NodeJS.Timeout; })) calls fn() synchronously and can mask event-loop ordering bugs; update the stub to schedule the callback as a microtask (use queueMicrotask(fn)) while preserving the return type (NodeJS.Timeout) so the stub better matches real timer semantics and avoids hiding ordering issues in the scraper tests.src/lib/currency.test.ts (1)
30-33: Brittle assertions — flagging as low priority.
CURRENCIES[0].code === "EUR"andlength >= 30bind tests tightly to dataset ordering and size. If the source is reordered or trimmed, these fail without a meaningful behavioral regression. Consider assertingCURRENCIES.find(c => c.code === "EUR")instead of index 0.Also applies to: 61-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/currency.test.ts` around lines 30 - 33, The test is brittle because it assumes EUR is at index 0 and that CURRENCIES length is >= 30; update the assertions in the spec(s) that reference CURRENCIES (e.g., the "includes EUR as the first entry" test and the other test around lines 61-63) to locate the currency by value instead of by index—use CURRENCIES.find(c => c.code === "EUR") (or similar) and assert the found object is defined and has the expected symbol, rather than asserting on CURRENCIES[0], and remove/replace any rigid length checks with assertions about required entries existing.src/types/station.test.ts (1)
18-25: Useful tripwire, but fragile if the union grows.
toHaveLength(16)will fail any timeFuelTypeis extended, forcing an intentional test update. That's the goal here, so it's fine — just make sure contributors know they should bump this when adding new fuel codes (consider a brief comment).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/types/station.test.ts` around lines 18 - 25, The test in the "FuelType accepts all known fuel codes" case currently asserts expect(fuels).toHaveLength(16) which is intentionally fragile; add a brief inline comment above or next to that assertion explaining this is a deliberate tripwire and that contributors must update the expected length when adding new members to the FuelType union (reference the FuelType union and the test case name so future editors know where to bump it). Ensure the comment mentions updating the 16 constant whenever FuelType is extended.src/scrapers/germany.test.ts (1)
12-26: Env-restoration via object reassignment is fine here, worth a note.
ORIG_ENV = process.envcaptures the reference; line 19 replacesprocess.envwith a fresh object, leaving the original intact, andafterEachrestores it. It works, butprocess.envproxies child-process env vars in Node — tests that spawn subprocesses with this pattern can surprise future contributors. Not a blocker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/germany.test.ts` around lines 12 - 26, ORIG_ENV currently holds a reference to process.env which risks surprises for subprocesses; change the setup to clone the env and restore it safely: in beforeEach set ORIG_ENV = { ...process.env } and then assign process.env = { ...ORIG_ENV, TANKERKOENIG_API_KEY: "test-key-123" } (or use Object.assign(process.env, {...}) to mutate the existing proxy), and in afterEach restore by mutating rather than replacing (e.g., Object.keys(process.env).forEach(k => delete process.env[k]); Object.assign(process.env, ORIG_ENV)); update references in beforeEach/afterEach around ORIG_ENV, setTimeout stubbing, and vi.restoreAllMocks/vi.resetModules accordingly.src/scrapers/norway.test.ts (2)
25-28:vi.stubGlobal("fetch", ...)isn't unstubbed.
vi.restoreAllMocks()doesn't restore globals set viastubGlobal. Addvi.unstubAllGlobals()inafterEachto avoid bleed into other files sharing a worker.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/norway.test.ts` around lines 25 - 28, The test teardown in afterEach currently calls vi.restoreAllMocks() and vi.resetModules() but does not unstub globals created with vi.stubGlobal (e.g., the fetch stub), which can leak into other tests; update the afterEach callback to also call vi.unstubAllGlobals() so any globals stubbed via vi.stubGlobal("fetch", ...) are removed—locate the afterEach block in the test file containing vi.restoreAllMocks() / vi.resetModules() and add vi.unstubAllGlobals() to the sequence.
12-18:node:cryptois fully replaced with onlycreateHash.If the scraper (or any transitive import) touches another
node:cryptoexport (e.g.randomUUID,randomBytes), it'll beundefinedat runtime. Safer to preserve the original module:♻️ Proposed fix
-vi.mock("node:crypto", () => ({ - createHash: () => ({ - update: () => ({ - digest: () => "mocked-md5-hash", - }), - }), -})); +vi.mock("node:crypto", async (importOriginal) => { + const actual = await importOriginal<typeof import("node:crypto")>(); + return { + ...actual, + createHash: () => ({ + update: () => ({ digest: () => "mocked-md5-hash" }), + }), + }; +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/norway.test.ts` around lines 12 - 18, The test currently replaces the entire node:crypto module with only createHash which will break any use of other exports like randomUUID/randomBytes; update the mock in src/scrapers/norway.test.ts to preserve the real module and override only createHash (use vi.importActual('node:crypto') or similar to get the original exports, spread them and replace createHash with the mocked implementation), ensuring other crypto functions remain defined at runtime.src/scrapers/base.test.ts (1)
185-201: Title promises ADBLUE but suite doesn't test it.Either drop
ADBLUEfrom the description or add an assertion for it — easy to lose track of later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/base.test.ts` around lines 185 - 201, The test description mentions ADBLUE but the test data doesn't include it; add an ADBLUE price to scraper.mockPrices and update the expected upsert count accordingly. Specifically, in this test case (the "allows alt fuel prices (H2, CNG, LNG, ADBLUE) with special range" it block) add a valid makePrice("s1", "ADBLUE" as RawFuelPrice["fuelType"], <validValue>, "EUR") entry (choose a value within the allowed range, e.g., 0.50) to scraper.mockPrices and update expect(result.pricesUpserted).toBe(...) to reflect the additional valid price.src/scrapers/ireland.test.ts (1)
10-10:setTimeoutstub fires synchronously and ignores delay args.Calling
fn()inline instead of deferring can change scheduling behavior (e.g., recursion depth, microtask ordering) compared to real delayed execution. If the scraper ever recurses throughsetTimeout, this could stack-overflow the test or hide ordering bugs. ConsiderqueueMicrotask(fn)(orsetImmediate) to preserve async ordering while still skipping the delay.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/ireland.test.ts` at line 10, The current test stub uses vi.stubGlobal("setTimeout", (fn: () => void) => { fn(); ... }) which calls the callback synchronously and discards delay, altering async scheduling and risking recursion/stack-overflow; change the stub to schedule the callback asynchronously (e.g., invoke the callback via queueMicrotask(fn) or setImmediate(fn) and still return a mock NodeJS.Timeout) so the test preserves microtask/macrotask ordering while skipping real delays — update the stub that references "setTimeout" in the test file accordingly.src/app/api/route-detour/route.test.ts (2)
56-80: Assertion ondetourMinsign is too loose.
getRouteDurationis mocked to return4200androuteDurationis3600, so the detour should be ~(4200 - 3600)/60 = 10minutes. Asserting only>= 0lets off-by-scale bugs (e.g., seconds vs minutes, or wrong sign) slip through. Consider asserting a concrete numeric value (toBeCloseTo(10, 1)).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/route-detour/route.test.ts` around lines 56 - 80, The test currently asserts result1.detourMin is >= 0 which is too weak; since getRouteDuration is mocked to 4200 and routeDuration is 3600 the expected detour in minutes is about (4200-3600)/60 = 10, so update the assertion in the POST route test (route.test.ts) to check a concrete value (e.g., expect(result1.detourMin).toBeCloseTo(10, 1)) to catch scale/sign errors when mocking getRouteDuration.
17-23: Dead parameter:abortedis never exercised.The
abortedflag inmakeRequestis never passedtrueby any test, so abort-handling in the streaming route has no coverage here. Either drop the parameter or add a test that passesaborted: trueto verify the stream shuts down cleanly on client disconnect.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/route-detour/route.test.ts` around lines 17 - 23, The helper makeRequest has an unused aborted parameter so abort-handling in the streaming route lacks coverage; update tests to exercise it by creating a request that simulates client disconnect (pass aborted: true to makeRequest) so the signal property uses AbortSignal.abort() and assert the route cleans up/ends the stream, or if you prefer to remove unused API surface, delete the aborted parameter and related AbortSignal.abort() usage from makeRequest and adjust tests to construct AbortController signals explicitly; locate makeRequest and the tests that call it to implement the change.src/app/api/route-stations/route.test.ts (1)
84-102: Tighten EV-path assertion.The test name says "without price join" but only checks that
properties.priceis undefined on the output — which is also what happens on any row withprice: null. To actually verify the EV SQL path, assert on the generated SQL (assrc/app/api/stations/route.test.tsdoes):const sqlArg = vi.mocked(prisma.$queryRawUnsafe).mock.calls[0][0] as string; expect(sqlArg).toContain("station_type"); expect(sqlArg).not.toMatch(/join\s+.*price/i);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/route-stations/route.test.ts` around lines 84 - 102, The test should verify the EV SQL path by asserting on the SQL sent to prisma.$queryRawUnsafe rather than relying only on output price; after mocking prisma.$queryRawUnsafe and before calling POST (or immediately after), capture vi.mocked(prisma.$queryRawUnsafe).mock.calls[0][0] and assert that the SQL string contains "station_type" and does not match a price join (e.g., does not match /join\s+.*price/i); update the "queries EV stations without price join" test to include these assertions referencing prisma.$queryRawUnsafe, POST, makeRequest, validBody, and mockStationRow so the test fails if the EV query uses a price join.src/scrapers/moldova.test.ts (1)
68-72: Nit: price currency assertion usesprices[0].
prices[0]depends on insertion order. If the scraper's fuel iteration changes, this silently asserts a different fuel's currency. Consider asserting currency on every price (e.g.,prices.every(p => p.currency === "MDL")) to make intent explicit and order-independent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scrapers/moldova.test.ts` around lines 68 - 72, The currency assertion currently checks prices[0] which is order-dependent; update the test in moldova.test.ts to assert currency for each price in the prices array (reference: the prices variable in the test) by replacing the single-index check with an order-independent assertion such as verifying prices.every(p => p.currency === "MDL") or mapping currencies and comparing to an expected array, so the test no longer relies on insertion order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/api/route-stations/route.test.ts`:
- Around line 138-148: The test "returns 400 when coordinates have fewer than 2
points" is dropping corridorKm so the 400 may be due to a missing required field
instead of short coordinates; update the test to start from validBody and only
override geometry (use something like const body = { ...validBody, geometry: {
type: "LineString", coordinates: [[-3.7, 40.4]] } }) before calling POST,
ensuring corridorKm remains present so the failure is caused by
coordinates.length < 2; locate this change around the POST import and
makeRequest usage in the test file (route.test.ts) and adjust assertions if
needed.
In `@src/lib/theme.test.ts`:
- Around line 1-25: The test currently asserts local constants instead of
exercising the actual module; fix by extracting the type and URL constants into
a plain TypeScript module (e.g., move the Theme type and the map style strings
into a non-"use client" file like theme.ts) and update src/lib/theme.test.ts to
import the real exports (Theme, LIGHT_STYLE, DARK_STYLE) and assert against
those imports so the test actually validates the module surface rather than
tautological local variables.
In `@src/scrapers/argentina.test.ts`:
- Around line 55-60: The test and scraper mapping use a non-canonical fuel code
"E5_PREMIUM" for "Nafta (premium) de más de 95 Ron"; update the scraper mapping
that maps the raw product name "Nafta (premium) de más de 95 Ron" to return the
canonical fuel id "E5_98" instead of "E5_PREMIUM", and update the test assertion
that currently looks for prices.find((p) => p.fuelType === "E5_PREMIUM") to
expect "E5_98"; ensure any other code paths or tests that reference "E5_PREMIUM"
for this product are updated to the canonical "E5_98".
In `@src/scrapers/austria.test.ts`:
- Around line 131-138: The test currently checks that the cheapest diesel entry
for stationExternalId "200" is <= 1.5 which doesn't guarantee dedup-by-cheapest;
update the assertion in the test that calls scraper.fetch() (the dieselPrices
array and its selection) to assert the exact expected cheapest value
(expect(dieselPrices[0].price).toBe(1.4)) so the test fails if the scraper keeps
the first item instead of the true minimum; leave the rest of the test (filter
by stationExternalId "200" and fuelType "B7", length check) intact.
In `@src/scrapers/finland.test.ts`:
- Around line 15-22: Move origSetTimeout to module scope so it captures the real
global setTimeout once (not the stubbed version), update the beforeEach to stub
via vi.stubGlobal("setTimeout", (...args) => origSetTimeout(...args)) so the
stub accepts/rest-forwards all arguments, and in afterEach call
vi.unstubAllGlobals() in addition to vi.restoreAllMocks()/vi.resetModules() to
properly restore the global stub; reference symbols: origSetTimeout, beforeEach,
afterEach, vi.stubGlobal, vi.unstubAllGlobals.
In `@src/scrapers/italy.test.ts`:
- Around line 7-14: The afterEach hook currently calls vi.restoreAllMocks() and
vi.resetModules() but does not undo the global stub created in beforeEach via
vi.stubGlobal("fetch"); add a call to vi.unstubAllGlobals() inside the afterEach
hook (after restoreAllMocks/resetModules or just inside the same callback) to
ensure the stubbed fetch global is removed and prevents cross-test pollution.
In `@src/scrapers/mexico.test.ts`:
- Around line 55-61: Update the fetch mock in mexico.test.ts to match endpoints
precisely: replace the substring check in vi.mocked(fetch).mockImplementation
(currently using urlStr.includes("places")) with exact URL path matching against
the PLACES_URL and PRICES_URL used in mexico.ts (or compare new
URL(urlStr).pathname and check it endsWith "/publicaciones/places" or
"/publicaciones/prices"); ensure the mock returns placesXml only for the exact
places endpoint and pricesXml only for the exact prices endpoint so other URLs
won't be mis-routed.
In `@src/scrapers/netherlands.test.ts`:
- Around line 113-138: The test is asserting that the scraper drops unknown
fuelType "HYDROGEN" but per canonical IDs we should map it to "H2"; update the
NetherlandsScraper fuel mapping used in fetch (or its helper that translates API
fuelType values) to include "HYDROGEN" → "H2" (or normalize "HYDROGEN" to "H2")
and then change the test to expect a price with fuelType "H2" instead of
asserting it was skipped; ensure the mapping logic lives in the scraper's
conversion utility used by NetherlandsScraper.fetch so future inputs are
normalized to the EU harmonized codes (E5, E10, E5_98, B7, B7_PREMIUM, LPG, CNG,
H2).
In `@src/scrapers/romania.test.ts`:
- Around line 67-72: Update the Romania scraper test to lock in the confirmed
premium mappings by adding explicit assertions that prices include entries for
fuelType "E5_98" and "B7_PREMIUM" (use the existing prices array and find by
fuelType to check price values), and address the AdBlue mapping by either
removing/ignoring any price with fuelType "ADBLUE"/"AdBlue" from the prices
array before asserting or updating the scraper to omit AdBlue so tests no longer
expect a non-harmonised "ADBLUE" code; refer to the test's prices variable and
the fuelType string literals ("E5_98", "B7_PREMIUM", "ADBLUE"/"AdBlue") when
making the change.
In `@src/scrapers/serbia.test.ts`:
- Around line 19-22: The test teardown in the afterEach block only calls
vi.restoreAllMocks() and vi.resetModules(), which leaves globals stubbed by
vi.stubGlobal (like fetch/setTimeout) leaking; update the afterEach to also call
vi.unstubAllGlobals() so global stubs are removed between tests (refer to the
afterEach function and the vi.restoreAllMocks / vi.resetModules calls and add
vi.unstubAllGlobals() to that sequence).
In `@src/scrapers/slovenia.test.ts`:
- Line 74: Update the canonical fuel-code guideline for src/scrapers/**/*.ts to
explicitly include HVO (matching the type defined in src/types/station.ts and
emitted by multiple scrapers) or explicitly document that HVO is treated as a
renewable/advanced fuel if you intend a separate category; locate the guideline
text (the list containing E5, E10, E5_98, B7, B7_PREMIUM, LPG, CNG, H2) and add
"HVO" to the list or add a clarifying sentence explaining the exception, then
run/verify tests like the expectation in src/scrapers/slovenia.test.ts that
assert HVO prices.
In `@src/scrapers/sweden.test.ts`:
- Around line 132-134: The test pins a non-canonical fuel code "HVO" (hvoPrice /
preemPrices.find) which violates the canonical EU fuel IDs used by the scrapers;
update the scraper in sweden.ts to map any HVO/HVO-blended diesel output to one
of the canonical IDs (B7 or B7_PREMIUM) or drop it entirely (search for the
mapping logic that sets fuelType to "HVO"), then change the test assertion to
look for the canonical ID (e.g., find fuelType === "B7" and assert its price) so
the test no longer hard-codes the non-standard "HVO" code.
- Around line 136-138: The FUEL_TYPE_MAP incorrectly maps fuelTypeId 9 (E‑85) to
"E10"; update sweden.ts to remove or change the [9, "E10"] entry so E‑85 is not
reported as an EU harmonized code (either omit the mapping or map to a sentinel
like undefined/"UNKNOWN"/null), and ensure the scraper logic that uses
FUEL_TYPE_MAP (the code that translates fuelTypeId → FuelType) skips or rejects
unmapped IDs; then update the test in sweden.test.ts to stop asserting an "E10"
price for fuelTypeId 9 (adjust the expectation to check that no mapping exists
or that the price is excluded).
In `@src/scrapers/turkey.test.ts`:
- Around line 6-27: Add a call to vi.unstubAllGlobals() in the afterEach cleanup
to undo the vi.stubGlobal("fetch", ...) used in the suite; update the afterEach
in this test to call vi.unstubAllGlobals() before vi.resetModules(), keeping
vi.restoreAllMocks(), and ensure the TurkeyScraper import/test still uses the
same behavior; also consider extracting the repeated pattern into a shared
parameterized helper (e.g., testFueloDelegation) so individual tests like the
TurkeyScraper suite simply call that helper with modulePath "./turkey",
exportName "TurkeyScraper", country "TR", source "fuelo_tr", subdomain "tr", and
currency "TRY".
---
Nitpick comments:
In `@src/app/api/route-detour/route.test.ts`:
- Around line 56-80: The test currently asserts result1.detourMin is >= 0 which
is too weak; since getRouteDuration is mocked to 4200 and routeDuration is 3600
the expected detour in minutes is about (4200-3600)/60 = 10, so update the
assertion in the POST route test (route.test.ts) to check a concrete value
(e.g., expect(result1.detourMin).toBeCloseTo(10, 1)) to catch scale/sign errors
when mocking getRouteDuration.
- Around line 17-23: The helper makeRequest has an unused aborted parameter so
abort-handling in the streaming route lacks coverage; update tests to exercise
it by creating a request that simulates client disconnect (pass aborted: true to
makeRequest) so the signal property uses AbortSignal.abort() and assert the
route cleans up/ends the stream, or if you prefer to remove unused API surface,
delete the aborted parameter and related AbortSignal.abort() usage from
makeRequest and adjust tests to construct AbortController signals explicitly;
locate makeRequest and the tests that call it to implement the change.
In `@src/app/api/route-stations/route.test.ts`:
- Around line 84-102: The test should verify the EV SQL path by asserting on the
SQL sent to prisma.$queryRawUnsafe rather than relying only on output price;
after mocking prisma.$queryRawUnsafe and before calling POST (or immediately
after), capture vi.mocked(prisma.$queryRawUnsafe).mock.calls[0][0] and assert
that the SQL string contains "station_type" and does not match a price join
(e.g., does not match /join\s+.*price/i); update the "queries EV stations
without price join" test to include these assertions referencing
prisma.$queryRawUnsafe, POST, makeRequest, validBody, and mockStationRow so the
test fails if the EV query uses a price join.
In `@src/lib/currency.test.ts`:
- Around line 30-33: The test is brittle because it assumes EUR is at index 0
and that CURRENCIES length is >= 30; update the assertions in the spec(s) that
reference CURRENCIES (e.g., the "includes EUR as the first entry" test and the
other test around lines 61-63) to locate the currency by value instead of by
index—use CURRENCIES.find(c => c.code === "EUR") (or similar) and assert the
found object is defined and has the expected symbol, rather than asserting on
CURRENCIES[0], and remove/replace any rigid length checks with assertions about
required entries existing.
In `@src/scrapers/australia.test.ts`:
- Around line 60-76: The test is missing an assertion for the canonical fuel
code on the price object; update the Australia scraper test (after finding
caltexPrice from prices returned by scraper.fetch()) to assert
caltexPrice!.fuelType equals the EU harmonized code your scraper emits for that
pump (e.g., "E10" or the correct ULP mapping) so regressions in fuel-type
mapping are caught; reference the caltexPrice variable and the prices array when
adding the expect assertion and follow the project guideline to use codes like
E5, E10, E5_98, B7, B7_PREMIUM, LPG, CNG, or H2.
- Line 10: The test currently stubs setTimeout globally via
vi.stubGlobal("setTimeout", ...) which interferes with Vitest internals; replace
that stub with vi.useFakeTimers({ shouldAdvanceTime: true }) at the start of the
test setup (remove the vi.stubGlobal call) and add vi.useRealTimers() in an
afterEach hook to restore real timers; ensure the new fake-timer setup preserves
the scraper's delay semantics and that teardown always runs to avoid leaking
fake timers.
In `@src/scrapers/austria.test.ts`:
- Line 10: The current test stub for setTimeout (vi.stubGlobal("setTimeout",
(fn: () => void) => { fn(); return 0 as unknown as NodeJS.Timeout; })) calls
fn() synchronously and can mask event-loop ordering bugs; update the stub to
schedule the callback as a microtask (use queueMicrotask(fn)) while preserving
the return type (NodeJS.Timeout) so the stub better matches real timer semantics
and avoids hiding ordering issues in the scraper tests.
In `@src/scrapers/base.test.ts`:
- Around line 185-201: The test description mentions ADBLUE but the test data
doesn't include it; add an ADBLUE price to scraper.mockPrices and update the
expected upsert count accordingly. Specifically, in this test case (the "allows
alt fuel prices (H2, CNG, LNG, ADBLUE) with special range" it block) add a valid
makePrice("s1", "ADBLUE" as RawFuelPrice["fuelType"], <validValue>, "EUR") entry
(choose a value within the allowed range, e.g., 0.50) to scraper.mockPrices and
update expect(result.pricesUpserted).toBe(...) to reflect the additional valid
price.
In `@src/scrapers/denmark.test.ts`:
- Around line 285-286: The assertion relies on array order; instead of indexing
prices[0], locate the B7 entry by using a lookup like prices.find(p =>
p.fuelType === "B7") and assert its currency equals "DKK" (and optionally assert
the find result is defined). Update the test in denmark.test.ts to reference the
found object rather than positional indexing to make the intent explicit and
robust.
- Around line 359-414: The test currently only asserts callCount > 1 which is
weak; update the "falls back to DrivstoffAppen on 401 from Fuelprices.dk" test
(using DenmarkScraper) to explicitly assert that the DrivstoffAppen auth
endpoint ("authorization-sessions") and the DrivstoffAppen stations endpoint
("/stations") were invoked; you can inspect vi.mocked(fetch).mock.calls (or
capture booleans in the mock implementation) and add two expects verifying at
least one fetch call string includes "authorization-sessions" and one includes
"/stations" to ensure the fallback path was actually exercised.
In `@src/scrapers/germany.test.ts`:
- Around line 12-26: ORIG_ENV currently holds a reference to process.env which
risks surprises for subprocesses; change the setup to clone the env and restore
it safely: in beforeEach set ORIG_ENV = { ...process.env } and then assign
process.env = { ...ORIG_ENV, TANKERKOENIG_API_KEY: "test-key-123" } (or use
Object.assign(process.env, {...}) to mutate the existing proxy), and in
afterEach restore by mutating rather than replacing (e.g.,
Object.keys(process.env).forEach(k => delete process.env[k]);
Object.assign(process.env, ORIG_ENV)); update references in beforeEach/afterEach
around ORIG_ENV, setTimeout stubbing, and vi.restoreAllMocks/vi.resetModules
accordingly.
In `@src/scrapers/ireland.test.ts`:
- Line 10: The current test stub uses vi.stubGlobal("setTimeout", (fn: () =>
void) => { fn(); ... }) which calls the callback synchronously and discards
delay, altering async scheduling and risking recursion/stack-overflow; change
the stub to schedule the callback asynchronously (e.g., invoke the callback via
queueMicrotask(fn) or setImmediate(fn) and still return a mock NodeJS.Timeout)
so the test preserves microtask/macrotask ordering while skipping real delays —
update the stub that references "setTimeout" in the test file accordingly.
In `@src/scrapers/luxembourg.test.ts`:
- Around line 11-14: The test teardown currently calls afterEach which runs
vi.restoreAllMocks() and vi.resetModules(), but it misses vi.unstubAllGlobals()
so globals stubbed with vi.stubGlobal may leak; update the afterEach cleanup to
also call vi.unstubAllGlobals() (alongside vi.restoreAllMocks() and
vi.resetModules()) to mirror other tests like australia.test.ts and ensure
globals stubbed in beforeEach are fully removed between tests.
In `@src/scrapers/moldova.test.ts`:
- Around line 68-72: The currency assertion currently checks prices[0] which is
order-dependent; update the test in moldova.test.ts to assert currency for each
price in the prices array (reference: the prices variable in the test) by
replacing the single-index check with an order-independent assertion such as
verifying prices.every(p => p.currency === "MDL") or mapping currencies and
comparing to an expected array, so the test no longer relies on insertion order.
In `@src/scrapers/norway.test.ts`:
- Around line 25-28: The test teardown in afterEach currently calls
vi.restoreAllMocks() and vi.resetModules() but does not unstub globals created
with vi.stubGlobal (e.g., the fetch stub), which can leak into other tests;
update the afterEach callback to also call vi.unstubAllGlobals() so any globals
stubbed via vi.stubGlobal("fetch", ...) are removed—locate the afterEach block
in the test file containing vi.restoreAllMocks() / vi.resetModules() and add
vi.unstubAllGlobals() to the sequence.
- Around line 12-18: The test currently replaces the entire node:crypto module
with only createHash which will break any use of other exports like
randomUUID/randomBytes; update the mock in src/scrapers/norway.test.ts to
preserve the real module and override only createHash (use
vi.importActual('node:crypto') or similar to get the original exports, spread
them and replace createHash with the mocked implementation), ensuring other
crypto functions remain defined at runtime.
In `@src/scrapers/serbia.test.ts`:
- Around line 255-298: The test title claims both coordinates are skipped but
the fixture only covers Latitude === 0; update the test for SerbiaScraper/fetch
to cover the Longitude === 0 case too by adding a second station object in
zeroCoordStation with a non-zero Latitude and Longitude: 0 (or alternatively
adjust the assertion/comment to reflect only Latitude is filtered); ensure the
mocked nisgazprom response includes both items and the final
expect(stations).toHaveLength(0) still holds so regressions that only filter
latitude or longitude are caught.
- Around line 14-17: The test currently overrides globalThis.setTimeout via
origSetTimeout which affects all timers; instead switch to Vitest's timer
utilities or inject a sleep: replace the global stub with vi.useFakeTimers() in
the test setup, call vi.runAllTimersAsync() (or
advanceTimersToNext/advanceTimersByTime as appropriate) around the dynamic
import or the code that triggers delays, and restore timers with
vi.useRealTimers() after; alternatively update the scraper to accept an
injectable delay/sleep function and pass a no-wait stub from the test rather
than touching globalThis.setTimeout (referencing origSetTimeout,
globalThis.setTimeout, vi.useFakeTimers, vi.runAllTimersAsync, and the scraper's
sleep parameter).
- Around line 349-394: Update the "handles cenagoriva.rs page errors gracefully"
test to assert that the scraper logs the upstream 500 error: spy on the logger
used by SerbiaScraper (e.g., console.warn or console.error) before calling
scraper.fetch(), exercise the mocked fetch that returns the 500 for
cenagoriva.rs, then assert the spy was called (and optionally that the message
contains "cenagoriva" or "500"); finally restore the spy/mock after the
assertion so subsequent tests are unaffected. Ensure you reference the
SerbiaScraper.fetch behavior and the cenagoriva.rs fetch case when adding the
assertion.
In `@src/scrapers/sweden.test.ts`:
- Line 155: The test's URL matching using url.includes("/stations") is fragile
and can misroute requests; update the mock predicate(s) (the conditional(s) that
currently use url.includes("/stations")) to match a more specific fragment such
as checking the request pathname equals "/stations" or matching the query param
"countryId=2" (or another exact identifier used in the test) so it won't collide
with auth endpoints; apply the same change to the other similar occurrences in
this test file (the other url.includes("/stations") conditions) to ensure
deterministic routing.
- Line 272: The tests use fragile substring assertions like await
expect(scraper.fetch()).rejects.toThrow("HTTP 503") and should instead assert a
stable pattern or the full message; update the failing assertions in
src/scrapers/sweden.test.ts (the expect(...).rejects.toThrow calls around
scraper.fetch()) to either compare against the full error string thrown by the
code (e.g., the complete "DrivstoffAppen stations API returned HTTP 503: ..."
message) or use a precise regex with toThrow(/DrivstoffAppen stations API
returned HTTP 503/) and likewise replace the "auth failed" substring check with
a full-message or regex matching "DrivstoffAppen auth failed: HTTP \\d+" so test
intent is robust to wording changes.
In `@src/types/station.test.ts`:
- Around line 18-25: The test in the "FuelType accepts all known fuel codes"
case currently asserts expect(fuels).toHaveLength(16) which is intentionally
fragile; add a brief inline comment above or next to that assertion explaining
this is a deliberate tripwire and that contributors must update the expected
length when adding new members to the FuelType union (reference the FuelType
union and the test case name so future editors know where to bump it). Ensure
the comment mentions updating the 16 constant whenever FuelType is extended.
In `@vitest.config.ts`:
- Line 18: The coverage exclusions in the vitest config currently exclude the
three React client provider modules (the exclude array entry containing
"src/lib/currency.tsx", "src/lib/i18n.tsx", "src/lib/theme.tsx"); add a short
actionable follow-up: add a TODO comment next to the exclude array and/or create
a tracked issue to enable jsdom for selective tests and restore these modules to
coverage once a jsdom-based test strategy is implemented, referencing the vitest
config exclude array and the three module paths so they can be re-included
later.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02202a1b-1487-4383-b562-52282fffb59c
📒 Files selected for processing (56)
.github/workflows/ci.ymlsrc/app/api/config/route.test.tssrc/app/api/geocode/route.test.tssrc/app/api/route-detour/route.test.tssrc/app/api/route-stations/route.test.tssrc/app/api/route/route.test.tssrc/app/api/stations/nearest/route.test.tssrc/app/api/stations/route.test.tssrc/app/api/stats/route.test.tssrc/lib/currency.test.tssrc/lib/db.test.tssrc/lib/i18n.test.tssrc/lib/theme.test.tssrc/lib/valhalla.test.tssrc/middleware.test.tssrc/scrapers/argentina.test.tssrc/scrapers/australia-nsw.test.tssrc/scrapers/australia.test.tssrc/scrapers/austria.test.tssrc/scrapers/base.test.tssrc/scrapers/belgium.test.tssrc/scrapers/bosnia.test.tssrc/scrapers/bulgaria.test.tssrc/scrapers/croatia.test.tssrc/scrapers/czech.test.tssrc/scrapers/denmark.test.tssrc/scrapers/estonia.test.tssrc/scrapers/finland.test.tssrc/scrapers/france.test.tssrc/scrapers/fuelo.test.tssrc/scrapers/germany.test.tssrc/scrapers/greece.test.tssrc/scrapers/hungary.test.tssrc/scrapers/ireland.test.tssrc/scrapers/italy.test.tssrc/scrapers/latvia.test.tssrc/scrapers/lithuania.test.tssrc/scrapers/luxembourg.test.tssrc/scrapers/mexico.test.tssrc/scrapers/moldova.test.tssrc/scrapers/netherlands.test.tssrc/scrapers/north-macedonia.test.tssrc/scrapers/norway.test.tssrc/scrapers/ocm.test.tssrc/scrapers/poland.test.tssrc/scrapers/portugal.test.tssrc/scrapers/romania.test.tssrc/scrapers/serbia.test.tssrc/scrapers/slovakia.test.tssrc/scrapers/slovenia.test.tssrc/scrapers/sweden.test.tssrc/scrapers/switzerland.test.tssrc/scrapers/turkey.test.tssrc/scrapers/uk.test.tssrc/types/station.test.tsvitest.config.ts
Comment on lines
+1
to
+25
| import { describe, it, expect } from "vitest"; | ||
|
|
||
| // theme.tsx is a "use client" module with React context. | ||
| // We can only test the exported type shape since the module | ||
| // requires React. We verify the module can be parsed by | ||
| // checking the type exports compile correctly. | ||
|
|
||
| describe("theme module types", () => { | ||
| it("Theme type accepts light and dark", () => { | ||
| // Type-level check — if this compiles, the types are correct | ||
| const light: "light" | "dark" = "light"; | ||
| const dark: "light" | "dark" = "dark"; | ||
| expect(light).toBe("light"); | ||
| expect(dark).toBe("dark"); | ||
| }); | ||
|
|
||
| it("map style URLs follow expected pattern", () => { | ||
| // Verify the known map style URLs that the module uses | ||
| const lightStyle = "https://tiles.openfreemap.org/styles/liberty"; | ||
| const darkStyle = "https://tiles.openfreemap.org/styles/dark"; | ||
| expect(lightStyle).toContain("openfreemap.org"); | ||
| expect(darkStyle).toContain("openfreemap.org"); | ||
| expect(lightStyle).not.toBe(darkStyle); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Tautological tests with no actual module coverage.
None of these assertions import anything from ./theme. They assert that locally-defined constants equal themselves, which exercises Vitest rather than the module under test. If theme.tsx cannot be loaded in the node env, consider either:
- Removing this file (it provides no signal), or
- Extracting the URL constants (and
Themetype) into a plain.tsmodule that can be imported and actually asserted against.
As-is, the file inflates the test count without adding coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/theme.test.ts` around lines 1 - 25, The test currently asserts local
constants instead of exercising the actual module; fix by extracting the type
and URL constants into a plain TypeScript module (e.g., move the Theme type and
the map style strings into a non-"use client" file like theme.ts) and update
src/lib/theme.test.ts to import the real exports (Theme, LIGHT_STYLE,
DARK_STYLE) and assert against those imports so the test actually validates the
module surface rather than tautological local variables.
Comment on lines
+55
to
+60
| const nafta = prices.find((p) => p.fuelType === "E5" && p.stationExternalId === ypf!.externalId); | ||
| expect(nafta!.price).toBeCloseTo(890.5, 1); | ||
| expect(nafta!.currency).toBe("ARS"); | ||
|
|
||
| expect(prices.find((p) => p.fuelType === "E5_PREMIUM")).toBeDefined(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what fuel code argentina.ts actually emits for premium nafta, and how other scrapers handle >95 RON
rg -nP -C3 '(E5_PREMIUM|E5_98|Nafta.*premium|de m.s de 95)' --type=tsRepository: GeiserX/Pumperly
Length of output: 32390
Update argentina scraper and test to use canonical fuel codes.
The mapping of "Nafta (premium) de más de 95 Ron" to E5_PREMIUM violates the coding guideline requiring E5, E10, E5_98, B7, B7_PREMIUM, LPG, CNG, H2 as canonical internal IDs. The test at line 59 perpetuates this non-canonical code.
Without additional product specifications, map premium >95 RON nafta to E5_98 (98 octane) as the closest canonical equivalent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scrapers/argentina.test.ts` around lines 55 - 60, The test and scraper
mapping use a non-canonical fuel code "E5_PREMIUM" for "Nafta (premium) de más
de 95 Ron"; update the scraper mapping that maps the raw product name "Nafta
(premium) de más de 95 Ron" to return the canonical fuel id "E5_98" instead of
"E5_PREMIUM", and update the test assertion that currently looks for
prices.find((p) => p.fuelType === "E5_PREMIUM") to expect "E5_98"; ensure any
other code paths or tests that reference "E5_PREMIUM" for this product are
updated to the canonical "E5_98".
Comment on lines
+15
to
+22
| const origSetTimeout = globalThis.setTimeout; | ||
| vi.stubGlobal("setTimeout", (fn: () => void, _ms?: number) => origSetTimeout(fn, 0)); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.restoreAllMocks(); | ||
| vi.resetModules(); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does vi.restoreAllMocks in Vitest revert globals set via vi.stubGlobal, or is vi.unstubAllGlobals required?
💡 Result:
vi.restoreAllMocks does NOT revert globals set via vi.stubGlobal. If you use vi.stubGlobal, you must call vi.unstubAllGlobals (or enable test.unstubGlobals in Vitest config) to restore those global values. Evidence: Vitest’s docs state that vi.stubGlobal “does not reset these globals” by default, and you can restore them by enabling the unstubGlobals option or calling vi.unstubAllGlobals manually. The vi.unstubAllGlobals API is specifically described as restoring global values changed with vi.stubGlobal. By contrast, vi.restoreAllMocks is described as restoring spies created with vi.spyOn (i.e., mock/spied functions), not globals stubbed via stubGlobal.
Citations:
- 1: https://vitest.dev/guide/mocking/globals
- 2: https://v3.vitest.dev/api/vi
- 3: https://main.vitest.dev/config/unstubglobals
- 4: https://v4.vitest.dev/api/vi
Restore stubbed globals properly and complete the stub signature.
vi.restoreAllMocks() doesn't revert globals set via vi.stubGlobal—you need vi.unstubAllGlobals(). Additionally, origSetTimeout is captured inside beforeEach, so each test captures the previously-stubbed version, causing nested wrappers. Move it to module scope and add vi.unstubAllGlobals() to afterEach. Also fix the stub signature to accept and forward rest arguments like the real setTimeout does.
♻️ Proposed fix
+const origSetTimeout = globalThis.setTimeout;
+
describe("FinlandScraper", () => {
beforeEach(() => {
vi.stubGlobal("fetch", vi.fn());
// Replace setTimeout to resolve immediately (avoids 115 * 100ms page delays)
- const origSetTimeout = globalThis.setTimeout;
- vi.stubGlobal("setTimeout", (fn: () => void, _ms?: number) => origSetTimeout(fn, 0));
+ vi.stubGlobal("setTimeout", (fn: (...a: unknown[]) => void, _ms?: number, ...args: unknown[]) =>
+ origSetTimeout(fn, 0, ...args),
+ );
});
afterEach(() => {
vi.restoreAllMocks();
+ vi.unstubAllGlobals();
vi.resetModules();
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scrapers/finland.test.ts` around lines 15 - 22, Move origSetTimeout to
module scope so it captures the real global setTimeout once (not the stubbed
version), update the beforeEach to stub via vi.stubGlobal("setTimeout",
(...args) => origSetTimeout(...args)) so the stub accepts/rest-forwards all
arguments, and in afterEach call vi.unstubAllGlobals() in addition to
vi.restoreAllMocks()/vi.resetModules() to properly restore the global stub;
reference symbols: origSetTimeout, beforeEach, afterEach, vi.stubGlobal,
vi.unstubAllGlobals.
| expect(prices.find((p) => p.fuelType === "B7")!.price).toBeCloseTo(1.449, 3); | ||
| expect(prices.find((p) => p.fuelType === "E5_98")!.price).toBeCloseTo(1.669, 3); | ||
| expect(prices.find((p) => p.fuelType === "LPG")!.price).toBeCloseTo(0.769, 3); | ||
| expect(prices.find((p) => p.fuelType === "HVO")!.price).toBeCloseTo(1.899, 3); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all fuel-type codes emitted by scrapers and test assertions
rg -nP --type=ts -C2 $'fuelType\\s*[:=]\\s*"[A-Z0-9_]+"' -g 'src/scrapers/**'
rg -nP --type=ts $'"HVO"' -g 'src/**'Repository: GeiserX/Pumperly
Length of output: 10583
Extend canonical fuel-code guideline to include HVO.
The coding guideline for src/scrapers/**/*.ts lists only (E5, E10, E5_98, B7, B7_PREMIUM, LPG, CNG, H2), but HVO is already defined in the type system (src/types/station.ts), actively emitted by multiple scrapers (Sweden, Spain, Slovenia, Norway, Ireland, Denmark), and exposed in the public API routes. Either update the guideline to explicitly include HVO as canonical, or clarify the rule's intent if HVO belongs in a separate category (e.g., renewable/advanced fuels).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scrapers/slovenia.test.ts` at line 74, Update the canonical fuel-code
guideline for src/scrapers/**/*.ts to explicitly include HVO (matching the type
defined in src/types/station.ts and emitted by multiple scrapers) or explicitly
document that HVO is treated as a renewable/advanced fuel if you intend a
separate category; locate the guideline text (the list containing E5, E10,
E5_98, B7, B7_PREMIUM, LPG, CNG, H2) and add "HVO" to the list or add a
clarifying sentence explaining the exception, then run/verify tests like the
expectation in src/scrapers/slovenia.test.ts that assert HVO prices.
Comment on lines
+132
to
+134
| const hvoPrice = preemPrices.find((p) => p.fuelType === "HVO"); | ||
| expect(hvoPrice).toBeDefined(); | ||
| expect(hvoPrice!.price).toBeCloseTo(25.99, 2); |
There was a problem hiding this comment.
HVO isn't an EU-harmonized fuel code.
The coding guideline for src/scrapers/**/*.ts restricts canonical fuel IDs to E5, E10, E5_98, B7, B7_PREMIUM, LPG, CNG, H2. This test pins the scraper's output to HVO, cementing a non-canonical code in the product. Either map HVO-blended diesel to B7/B7_PREMIUM (or drop it) in sweden.ts and update the assertion, or extend the canonical set deliberately with a documented exception — but don't lock in a drift via tests.
As per coding guidelines: "Use EU harmonized fuel type codes (E5, E10, E5_98, B7, B7_PREMIUM, LPG, CNG, H2) as canonical internal IDs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scrapers/sweden.test.ts` around lines 132 - 134, The test pins a
non-canonical fuel code "HVO" (hvoPrice / preemPrices.find) which violates the
canonical EU fuel IDs used by the scrapers; update the scraper in sweden.ts to
map any HVO/HVO-blended diesel output to one of the canonical IDs (B7 or
B7_PREMIUM) or drop it entirely (search for the mapping logic that sets fuelType
to "HVO"), then change the test assertion to look for the canonical ID (e.g.,
find fuelType === "B7" and assert its price) so the test no longer hard-codes
the non-standard "HVO" code.
Comment on lines
+136
to
+138
| const e10Price = prices.find((p) => p.fuelType === "E10"); | ||
| expect(e10Price).toBeDefined(); | ||
| expect(e10Price!.price).toBeCloseTo(14.99, 2); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e ts sweden --exec rg -nP -C1 'fuelTypeId|fuel_type|case\s+\d|: ?"E10"|: ?"E5"|: ?"B7"|: ?"HVO"' {}Repository: GeiserX/Pumperly
Length of output: 2231
🏁 Script executed:
rg -n "FUEL_TYPE_MAP" src/scrapers/sweden.ts -A 15Repository: GeiserX/Pumperly
Length of output: 1234
Map fuelTypeId: 9 to the correct EU harmonized code—not E10.
The mapping exists and confirms fuelTypeId: 9 → "E10", but the inline comment reveals the actual issue: it maps E-85 (85% ethanol) to E10 (10% ethanol). These are entirely different fuels. E-85 is not in the EU harmonized set, so either the scraper should reject it or map it to the correct code. Either way, shipping E-85 prices labeled as E10 will mislead users and skew price comparisons.
Current mapping (sweden.ts:51-59)
const FUEL_TYPE_MAP: ReadonlyMap<number, FuelType> = new Map([
[1, "B7"], // Diesel
[2, "E5"], // 95 Oktan
[3, "E5_98"], // 98 Oktan
[4, "B7"], // Frigårdsdiesel (duty-free diesel)
[7, "HVO"], // HVO 100
[8, "E5"], // 92 Oktan (lower octane gasoline; E5 closest match)
[9, "E10"], // E-85 (high ethanol blend)
]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/scrapers/sweden.test.ts` around lines 136 - 138, The FUEL_TYPE_MAP
incorrectly maps fuelTypeId 9 (E‑85) to "E10"; update sweden.ts to remove or
change the [9, "E10"] entry so E‑85 is not reported as an EU harmonized code
(either omit the mapping or map to a sentinel like undefined/"UNKNOWN"/null),
and ensure the scraper logic that uses FUEL_TYPE_MAP (the code that translates
fuelTypeId → FuelType) skips or rejects unmapped IDs; then update the test in
sweden.test.ts to stop asserting an "E10" price for fuelTypeId 9 (adjust the
expectation to check that no mapping exists or that the price is excluded).
- Use BigInt() instead of BigInt literals (0n, 3n) for ES2020 compat - Cast process.env for NODE_ENV assignment (read-only in strict types)
- Add vi.unstubAllGlobals() to afterEach in 6 test files - Use order-independent assertions (find/every vs index) - Strengthen Denmark 401-fallback test with URL assertions - Add ADBLUE to alt-fuel price range test - Tighten Austria diesel dedup to exact value - Fix route-stations 400 test to keep corridorKm - Remove unused aborted param from route-detour helper - Tighten detourMin assertion to toBeCloseTo(10) - Move origSetTimeout to module scope in Finland tests - Add tripwire comment on FuelType length assertion
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
1 participant
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.
Summary
What's covered
Coverage exclusions
src/generated/**(Prisma generated code)src/scrapers/cli.ts(CLI entry point)src/lib/currency.tsx,src/lib/i18n.tsx,src/lib/theme.tsx(React client context providers requiring jsdom)Test plan
Summary by CodeRabbit
Tests
Chores