Add unit test suites for jobplugins, dataset and strategus-analysis plugin functions - #2989
Open
ohdsi-trex wants to merge 9 commits into
Open
Add unit test suites for jobplugins, dataset and strategus-analysis plugin functions#2989ohdsi-trex wants to merge 9 commits into
ohdsi-trex wants to merge 9 commits into
Conversation
…t starting the server
ohdsi-trex
requested review from
LSriragavan,
SantanM,
brandantck,
jerome-ng,
khairul-syazwan,
maggie-li-yd,
p-hoffmann and
suwarnoong
as code owners
July 27, 2026 05:37
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Deno unit test coverage for several Trex plugin functions (jobplugins, dataset, strategus-analysis) using a shared in-repo test harness, and refactors the dataset plugin to make the router importable without pulling in the server bootstrap.
Changes:
- Added new Deno test suites for
strategus-analysis,jobplugins, anddatasetthat directly invoke Express route handlers with request/response doubles and stubs. - Introduced shared test utilities under
plugins/functions/_shared/testing/(HTTP doubles, router lookup helpers, Trex global shim, express-validator runner). - Extracted
DatasetRouterintoplugins/functions/dataset/router.tsand simplifieddataset/index.tsto bootstrap wiring.
Reviewed changes
Copilot reviewed 19 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| plugins/functions/strategus-analysis/tests/utils.test.ts | Adds unit test for getDummyDataset. |
| plugins/functions/strategus-analysis/tests/study-validation.test.ts | Adds middleware/service-level study validation characterization tests. |
| plugins/functions/strategus-analysis/tests/analysis-service.test.ts | Adds unit tests for analysis service repository interactions and enrichment behavior. |
| plugins/functions/strategus-analysis/tests/analysis-routes.test.ts | Adds route-handler tests for analysis endpoints and error mapping. |
| plugins/functions/strategus-analysis/tests/_setup.ts | Test setup: env priming + Trex global shim. |
| plugins/functions/strategus-analysis/deno.json | Adds deno task test and std imports for test assertions/mocking. |
| plugins/functions/strategus-analysis/deno.lock | Locks new std dependencies used by tests. |
| plugins/functions/jobplugins/tests/dqd-validators.test.ts | Adds express-validator chain tests for DQD DTO/query validation. |
| plugins/functions/jobplugins/tests/dqd-controller.test.ts | Adds controller route-chain tests (validators + handler) for key DQD endpoints. |
| plugins/functions/jobplugins/tests/_setup.ts | Test setup: SERVICE_ROUTES + DB env + Trex global shim. |
| plugins/functions/jobplugins/deno.json | Adds deno task test and std imports for test assertions/mocking. |
| plugins/functions/jobplugins/deno.lock | Locks new std dependencies used by tests. |
| plugins/functions/dataset/tests/dataset-router.test.ts | Adds dataset router tests for query parsing, happy path, and error paths. |
| plugins/functions/dataset/tests/_setup.ts | Test setup: SERVICE_ROUTES + DB env + Trex global shim. |
| plugins/functions/dataset/router.ts | Extracted DatasetRouter implementation into its own module for testability. |
| plugins/functions/dataset/index.ts | Bootstrap-only wiring for express app + router mount. |
| plugins/functions/dataset/deno.json | Adds deno task test and std imports for test assertions/mocking. |
| plugins/functions/dataset/deno.lock | Locks new std dependencies used by tests. |
| plugins/functions/_shared/testing/validator-helpers.ts | Shared helper to execute express-validator chains on mock requests. |
| plugins/functions/_shared/testing/trex-global.ts | Shared Trex ambient-global shim for tests. |
| plugins/functions/_shared/testing/router-helpers.ts | Shared router introspection helpers to locate handlers/chains. |
| plugins/functions/_shared/testing/http-doubles.ts | Shared Express-style request/response doubles for tests. |
| recursive: true, | ||
| }); | ||
| await fs.writeFile(localFilePath, buffer); | ||
| } catch (e) {} |
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
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
Adds
deno testunit suites to three plugin functions that previously had none:jobplugins,dataset, andstrategus-analysis. 42 tests, all passing.A shared harness under
plugins/functions/_shared/testing/supplies Express request/response doubles, aTrexambient-global shim, router lookup helpers, and an express-validator runner. Tests call route handlers directly with fakereq/resand stub collaborators (API clients, TypeORM repositories) via@std/testing/mock. No live HTTP, no live database.Production change — one file pair, testability only
dataset/index.tsdeclaredexport class DatasetRouterand calledapp.listen(8000)at module scope, so importing the class started a server and hung the test process. The class is extracted verbatim intodataset/router.ts;index.tsis now bootstrap-only.This is a pure move: the class body and import block are byte-identical to the original, and the registered route table was dumped before and after and matches exactly (8 layers, same order,
generateDatasetSchemamiddleware intact). No handler logic, status code, or response string changed.Two lazy-initialization changes considered during design were dropped as unnecessary — module-scope
Deno.envreads are handled by importing a_setup.tsfirst (ES modules evaluate in import order), and TypeORM'snew DataSource()does not connect until.initialize().Coverage
strategus-analysisjobpluginsdatasetEach test file was verified able to fail (assertion mutated, run, confirmed FAILED, restored).
Known limitations
--no-checkonstrategus-analysisandjobplugins. Both inherit pre-existing type errors that any importing test picks up:datasource.tspassesPG__PORT(a string) where TypeORM wants a number, andPortalServerAPI.ts:280references an out-of-scopepath. Fixing them is a production change, kept out of scope.datasettype-checks cleanly.jobpluginscontrollers and 2 of 8datasetroutes. Extending is mechanical repetition of the established shape.alp-trexrestart.Pre-existing issues found while writing these tests (not fixed here)
jobplugins/src/api/PortalServerAPI.ts:280—const pathis declared insidetrybut referenced incatch; any failure throwsReferenceErrorand destroys the original error.jobplugins/src/middlewares/DqdRequestValidatorMiddlewares.ts:25—.isUUID().notEmpty().withMessage(...)attaches the message only tonotEmpty, so a malformed UUID returns the unhelpful default"Invalid value".dataset/router.tshandles a missingAuthorizationheader two different ways on the same router:/cdm-schema/snapshot/metadataconstructs its API client outside thetry(unhandled rejection, no response), while/cohortsconstructs it inside (500). Neither returns 401.strategus-analysisvalidateStudyIdreturnstruewhen no token is supplied, so unauthenticated callers bypass study-existence validation.Each is characterized by a named test asserting actual behaviour. Fixing any is a behaviour change deserving its own decision.
Not included
The CI workflow that runs these suites (
.github/workflows/plugin-function-tests.yml) is written and committed locally but not in this PR — the available token lacks theworkflowOAuth scope. It needs to be pushed by someone with that scope, or added in a follow-up.