Skip to content

chore(test): remove legacy API test suite and related infrastructure - #1173

Merged
kriszyp merged 1 commit into
mainfrom
claude/cleanup-legacy-tests
Jun 9, 2026
Merged

chore(test): remove legacy API test suite and related infrastructure#1173
kriszyp merged 1 commit into
mainfrom
claude/cleanup-legacy-tests

Conversation

@kriszyp

@kriszyp kriszyp commented Jun 8, 2026

Copy link
Copy Markdown
Member

Summary

  • Removes the legacy integrationTests/apiTests/tests/ directory (29 test files + testSuite.mjs) — all content has been ported to self-contained *.test.mjs files now living alongside the data/utils dirs
  • Removes legacy shared utilities (utils/request.mjs, restart.mjs, schema.mjs, etc.) and the OCSP/CRL security utilities (used only by unported TLS tests)
  • Removes config/envConfig.mjs and .env.example (new tests get credentials from ctx.harper.admin, not env vars)
  • Removes package.json script test:integration:api-tests
  • Removes three "Integration API Tests" CI jobs from integration-tests.yml (Node.js matrix, Bun disabled-job, Windows) — coverage now fully provided by the sharded Integration Tests jobs that glob all *.test.mjs files

⚠️ Depends on: PRs #761 (delete tests) and #778 (northwind tests) — merge those first to avoid the new test files conflicting.

TLS tests (24_ocspVerification, 26_crlVerification) were not in testSuite.mjs and required special mTLS/OpenSSL infrastructure; they are deferred for a future migration PR.

Test plan

🤖 Generated with Claude Code

All tests from the legacy testSuite.mjs have been ported to the
self-contained integration test framework (*.test.mjs files). Each
new test file starts its own Harper instance, eliminating shared state
and the sequential-dependency model of the old suite.

Removes:
- integrationTests/apiTests/tests/ — 29 legacy test files + testSuite.mjs
- integrationTests/apiTests/utils/ — legacy test utilities (request.mjs,
  restart.mjs, schema.mjs, etc.) and security/ (OCSP/CRL tools for
  unported TLS tests)
- integrationTests/apiTests/config/envConfig.mjs
- integrationTests/apiTests/.env.example
- package.json: test:integration:api-tests script
- .github/workflows/integration-tests.yml: Integration API Tests jobs
  (Node.js matrix, Bun, Windows) — coverage now provided by the sharded
  Integration Tests jobs that run all *.test.mjs files

TLS tests (24_ocspVerification, 26_crlVerification) were not in
testSuite.mjs and required special mTLS/OpenSSL infrastructure;
they are deferred for separate migration.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kriszyp
kriszyp requested a review from Ethan-Arrowood June 8, 2026 22:11
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@claude

claude Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

1. Prerequisite PRs are still open — coverage would silently drop if merged now

File: integrationTests/apiTests/tests/testSuite.mjs (deleted)
What: PRs #761 (delete-ops port) and #778 (Northwind/SQL port) are both still OPEN. This PR removes the only test runner for 3_sqlTests, 4_noSqlTests, 5_noSqlRoleTesting, 6_sqlRoleTesting, 7_jobsAndJobRoleTesting, 8_deleteTests, 8a_restartHdbToUpdateConfig, and 10_otherRoleTests. None of those have a *.test.mjs replacement in the current HEAD.
Why it matters: CI will not fail — the old test script is gone and the replacement tests don't exist yet. SQL queries, NoSQL CRUD, role-based data access, and delete operations would stop being exercised by CI without any alert.
Suggested fix: Merge #761 and #778 first (per the test plan in the PR body), then merge this PR. The test plan already says this; the blocker is that the prerequisites are not yet merged.

@kriszyp
kriszyp marked this pull request as draft June 8, 2026 22:29
@kriszyp

kriszyp commented Jun 8, 2026

Copy link
Copy Markdown
Member Author

Acknowledged — this is already documented in the PR description as a hard prerequisite ("⚠️ Depends on: PRs #761 and #778 — merge those first"). Converting to draft to make the ordering explicit and prevent accidental early merge.

— Claude

@kriszyp
kriszyp marked this pull request as ready for review June 8, 2026 22:33
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@kriszyp
kriszyp merged commit b4f8388 into main Jun 9, 2026
34 of 38 checks passed
@kriszyp
kriszyp deleted the claude/cleanup-legacy-tests branch June 9, 2026 03:51
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