Typescript Migration: Tests#1095
Conversation
…nner not identifying the error typescript file and finding temporary workaround during migration.
…ts. Confirmed using s3-utils.ts passes all tests
…ut input object in test to be a more complete StacItem to pass an expected logic gate
…adding types as necessary
… feat: some of the LLM converted tests
…910/type-test-finalize-typescript-migration
…910/type-test-finalize-typescript-migration
…-finalize-typescript-migration # Conflicts: # .nsprc
matthewhanson
left a comment
There was a problem hiding this comment.
Reviewed and verified — approving. This completes the TypeScript migration (#206).
Coverage preserved: counted test declarations both sides — 257 cases on main and 257 on this branch, zero per-file mismatches. The large line deltas (search-post -737, aggregate -672) are pure reformatting, not dropped tests.
"No business logic changes" holds up. The source edits are type annotations, casts, and non-null assertions. The two non-trivial spots are behavior-equivalent: the context-extension blocks refactored from if to object-spread, and two defensive if (!operatorsObject) return guards in database.ts that can't trigger (keys come from Object.keys). Nice wins: removing // @ts-nocheck from app.ts and flipping noImplicitAny: true.
Verified locally on the merged result (main + this branch): audit-prod ✓, lint ✓, typecheck (both tsconfig + tsconfig.test) ✓, check-openapi ✓, unit (69) ✓, system (171) ✓, build ✓.
Minor, non-blocking (fine to address in a follow-up):
@ts-ignore→@ts-expect-error(database.ts ×2, geo-utils.ts ×1) so they self-remove when no longer needed.types.tsnow importsOPfromdatabase.tswhiledatabase.tsimports fromtypes.ts— a circular reference. Safe here (type-onlytypeof OP, elided at emit, typecheck passes), butimport type/a shared constants module would be cleaner.tsconfig.test.jsonrelaxesnoImplicitAnyback tofalsefor tests — pragmatic, just a conscious trade-off.- Comment typo in
getCollections: "adding 'context' this to the Collections response".
I merged main into the branch to resolve the .nsprc conflict from #1118 (took main's now-empty .nsprc, since #1118's lockfile update cleared the advisories this PR was excluding).
Align the temporal-extent test with the completed TS migration (#1095), which set ava `extensions: ["ts"]`. As a lone `.js` file it only ran via the explicit glob in system-tests.sh and would be skipped by a plain `ava` invocation. Typed `t.context` via `StandUpResult` to match the other migrated system tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The TS test conversion (#1095) added href: '' to the 'handles assets without href' test to satisfy the Assets type, which turned it into a with-empty-href test that no longer matched its name. Provide an asset genuinely missing href (via a cast) so it exercises the intended missing-href branch of getProxiedAssets.
The TS test conversion (#1095) added href: '' to the 'handles assets without href' test to satisfy the Assets type, which turned it into a with-empty-href test that no longer matched its name. Provide an asset genuinely missing href (via a cast) so it exercises the intended missing-href branch of getProxiedAssets.
The TS test conversion (#1095) added href: '' to the 'handles assets without href' test to satisfy the Assets type, which turned it into a with-empty-href test that no longer matched its name. Provide an asset genuinely missing href (via a cast) so it exercises the intended missing-href branch of getProxiedAssets.
Related Issue(s):
Proposed Changes:
Converting the tests to Typescript. This was done with having Claude doing a major first pass and then reviewing myself, after doing all the other work of building and honing types through the other PRs of the typescript migration, and then let claude rip on the tests.
This is the final PR that completes the typescript migration of stac-server.
A significant amount of the diff is minor linting changes required by typescript, though some changes were also necessitated by changing
noImplicitAny: falsetotruewhich turned up some additional typescript errors around the repo that required some minor changes, particularly indatabase.ts. Keeping in line with the rest of the migration there are no changes to business logic.PR Checklist: