Skip to content

Typescript Migration: Tests#1095

Merged
matthewhanson merged 46 commits into
mainfrom
tr/drop-910/type-test-finalize-typescript-migration
Jun 20, 2026
Merged

Typescript Migration: Tests#1095
matthewhanson merged 46 commits into
mainfrom
tr/drop-910/type-test-finalize-typescript-migration

Conversation

@theodorehreuter

@theodorehreuter theodorehreuter commented Apr 23, 2026

Copy link
Copy Markdown
Collaborator

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: false to true which turned up some additional typescript errors around the repo that required some minor changes, particularly in database.ts. Keeping in line with the rest of the migration there are no changes to business logic.

PR Checklist:

  • [ x] I have added my changes to the CHANGELOG

…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
@theodorehreuter theodorehreuter changed the base branch from tr/drop-909/type-ingest-and-lambda-entry to main May 4, 2026 19:59
@matthewhanson matthewhanson added this pull request to the merge queue Jun 20, 2026
@matthewhanson matthewhanson removed this pull request from the merge queue due to a manual request Jun 20, 2026
…-finalize-typescript-migration

# Conflicts:
#	.nsprc

@matthewhanson matthewhanson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.ts now imports OP from database.ts while database.ts imports from types.ts — a circular reference. Safe here (type-only typeof OP, elided at emit, typecheck passes), but import type/a shared constants module would be cleaner.
  • tsconfig.test.json relaxes noImplicitAny back to false for 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).

@matthewhanson matthewhanson added this pull request to the merge queue Jun 20, 2026
Merged via the queue into main with commit c58ffb4 Jun 20, 2026
5 checks passed
matthewhanson added a commit that referenced this pull request Jun 20, 2026
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>
@jkeifer jkeifer mentioned this pull request Jul 1, 2026
1 task
jkeifer added a commit that referenced this pull request Jul 1, 2026
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.
jkeifer added a commit that referenced this pull request Jul 1, 2026
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.
jkeifer added a commit that referenced this pull request Jul 1, 2026
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants