Per maintainer review (RohitR311 on #1138): the repo has no existing
test infra or CI to run tests, so introducing jest here is out of
scope. Removes jest configs, test files, tsconfig.test.json, the
test:server script, and jest/ts-jest/@types/jest/jest-environment-jsdom
dependencies from both maxun-core and server.
output-validation.ts stays - it's a real production module (not test
code) extracted to fix the duplicated isPartial bug CodeRabbit flagged;
only its test file is dropped.
computeCrawlSearchPersistDelta advances lastPersistedResultCount as
soon as a delta is computed, before the write is confirmed. If
flushPersistenceBuffer exhausts its retries and drops the batch, that
data was never actually written, but the cursor had already moved
past it - so it was silently lost forever instead of being resent
with the next page's delta.
Roll the cursor back by the dropped delta's size when a batch is
dropped, so the next page recovers the lost items instead of skipping
them. Covered by two new tests exercising the real retry-exhaustion
path via a rejecting sequelize.transaction mock.
If triggerIntegrationUpdates threw, the catch(_) swallowed it and left
partialDataExtracted false even though hasData was true, so isPartial
was persisted incorrectly on the failure path.
Addresses CodeRabbit review on #1138 (both findings confirmed valid
against the actual code, not just the review's claim):
- hasData's scrapeSchema/scrapeList checks used `.length > 0`, but both
are always plain objects keyed by action name, never arrays -
`.length` on a plain object is undefined, so the check could never
be true. isPartial could never reflect a run whose only partial
output was scrapeSchema/scrapeList. Pre-existing bug, not introduced
by this PR, but copied into abortRun's check when extending it here.
- abortRun reloaded the run before checking for partial data but never
forced the interpreter's persistence buffer to flush first -
scrapeList isn't on the immediate-flush allowlist, so a buffered
scrapeList page could be invisible to the reload.
Both fixes now live in one place (hasPartialOutput /
flushReloadAndCheckPartialOutput in output-validation.ts) instead of
duplicated inline in task-runner.ts's two call sites, which is how the
same bug ended up in two places to begin with.
Verified with mutation testing: reintroduced the .length > 0 bug,
confirmed 3 tests fail; reintroduced the reload-before-flush ordering
bug, confirmed the ordering test fails; restored the fix, all 14 pass.
Rebasing onto the latest develop pulled in #1105's per-page screenshot
upload path, which added a BinaryOutputService import to Interpreter.ts
that attempts a real MinIO connection at module load. Mock it so these
tests stay hermetic instead of noisily (and potentially flakily)
failing to connect to a MinIO that isn't there in CI.
Same class of bug already fixed on the maxun-core side: `tsc -p
server/tsconfig.json` (the actual build:server script) was compiling
server/src/**/__tests__ into server/dist, since the base tsconfig's
include (src/**/*) picked them up with nothing excluding them. Found
by actually running the production build rather than trusting
--noEmit type-checks, which don't reveal what gets emitted.
Discovered while actually building maxun-core to verify the fix end-to-end:
running `npm run build` compiled with type errors (test files pulled into
the production tsconfig with no jest types, plus a pre-existing implicit-any
in concurrency.ts) and still silently emitted output anyway. Exclude
src/__tests__ from the shipped build and fix the implicit-any so `npm run
build` is actually clean, not just non-fatal.
server had no test infra at all (no test script, no jest config). Add
jest/ts-jest scoped to server/, and directly unit test
computeCrawlSearchPersistDelta/mergeCrawlDelta/mergeSearchDelta - the
new delta-slicing and append-merge logic - without needing to mock
the database or drive a full interpreter run.
Verified these tests actually catch the regression they're meant to:
temporarily reverted the slicing to send full arrays again and
confirmed 4 of the 11 tests fail, including the end-to-end one showing
the exact page duplication the fix prevents.
crawl/search deliver the full cumulative array on every page callback,
and persistDataToDatabase was writing that whole array to Postgres on
every single page - meaning page 1's data got re-sent on every write
for the rest of the run, an O(n^2) total-bytes cost that gets worse
the more pages a crawl visits (each carrying full page HTML).
Track how many items have already been persisted per action and slice
off just the new ones before they reach the persistence buffer, then
append (rather than overwrite) that delta into the existing DB state.
The in-memory cache and socket broadcast are untouched - only what
gets written to Postgres changes. Extracted as computeCrawlSearchPersistDelta/
mergeCrawlDelta/mergeSearchDelta so this can be tested directly.
maxun-core had no test infra at all (jest was declared in package.json
but never installed, no config, no test files). Add jest/ts-jest and a
jsdom-backed mock Page, and drive the real Interpreter through crawl
and search-scrape workflows to prove serializableCallback fires once
per page with a strictly growing snapshot - not once at the end - and
that a mid-run failure or abort still leaves prior pages recorded.
status alone doesn't distinguish a failed/aborted run that still has
usable partial data from one with nothing recovered - that signal only
existed transiently in socket/webhook payloads and was lost on reload.
Persist it as a real column, set from both the crash-catch path and
abortRun, and expose it via formatRunResponse.
Also fix abortRun's partial-data check, which only looked at
scrapeSchema/scrapeList and missed crawl/search entirely, and reload
the run before checking since the interpreter's persistence buffer
writes through a separate Run instance.
persistDataToDatabase only force-flushed the batched write immediately
for scrapeSchema; everything else, including per-page crawl/search
writes, sat in the buffer for up to 3s or 5 items before hitting
Postgres. A hard process kill inside that window would still lose
data. Extend the immediate-flush condition to crawl and search too.
The crawl and search-scrape actions accumulated every visited page into
a local array and only handed it to the persistence callback once,
after the whole page-by-page loop finished. If the run was killed,
timed out, or errored mid-loop, everything scraped so far was lost
even though it had already been extracted.
Persist the running snapshot after every page (success or failure),
matching the pattern scrapeList pagination already uses. Also stop the
search scrape loop promptly on abort, matching crawl's existing check.
amhsirak