mp_testservice.sas → mp_execute.sas (rename, same parameters) — updated 16 test files under sas/sasjs/** (all the *.test.sas files, tests/testsetup.sas, tests/testinit.sas, and their doxygen @li references).
mp_getddl.sas → mp_ds2ddl.sas — updated:
sas/sasjs/services/public/getddl.sas
sas/sasjs/services/admin/exportdb.sas
In v5 SHOWLOG now defaults to YES (was NO), so I added showlog=NO to both calls to preserve the previous behaviour (both append DDL to a fileref and stream it back; log noise isn't wanted).
mp_coretable.sas removed — sas/sasjs/tests/testsetup.sas now calls the standalone DDL macros directly:
%mddl_dc_locktable(libds=...) and %mddl_dc_difftable(libds=...) (exact equivalents, per the release notes).
Checked and unaffected: mp_abort type= removal (dc never passes type=), mf_getuniquelibref maxtries removal (not used), mcf_* insert_cmplib removal / wrap= default change (dc only uses
%mcf_getfmttype(wrap=YES), still valid).
Verification:
npx sasjs compile -t server succeeds, and the generated sasjsbuild/ output contains only the new macros
(mp_execute, mddl_dc_locktable, mp_ds2ddl) — no dangling references to the removed ones.
sasjs lint passes on all .sas files.
@sasjs/core is bumped to 5.0.0 in sas/package.json and all v5 breaking changes are addressed:
Breaking changes from the v5.0.0 release notes (https://github.com/sasjs/core/releases/tag/v5.0.0) and how dc was affected:
1. mp_testservice.sas → mp_execute.sas (rename, same parameters) — updated 16 test files under sas/sasjs/** (all the *.test.sas files, tests/testsetup.sas, tests/testinit.sas, and their doxygen @li references).
2. mp_getddl.sas → mp_ds2ddl.sas — updated:
- sas/sasjs/services/public/getddl.sas
- sas/sasjs/services/admin/exportdb.sas
In v5 SHOWLOG now defaults to YES (was NO), so I added showlog=NO to both calls to preserve the previous behaviour (both append DDL to a fileref and stream it back; log noise isn't wanted).
3. mp_coretable.sas removed — sas/sasjs/tests/testsetup.sas now calls the standalone DDL macros directly:
%mddl_dc_locktable(libds=...) and %mddl_dc_difftable(libds=...) (exact equivalents, per the release notes).
4. Checked and unaffected: mp_abort type= removal (dc never passes type=), mf_getuniquelibref maxtries removal (not used), mcf_* insert_cmplib removal / wrap= default change (dc only uses
%mcf_getfmttype(wrap=YES), still valid).
Verification:
- npx sasjs compile -t server succeeds, and the generated sasjsbuild/ output contains only the new macros
(mp_execute, mddl_dc_locktable, mp_ds2ddl) — no dangling references to the removed ones.
- sasjs lint passes on all .sas files.
Confirmed all three old macro names have zero remaining references across the entire repo (not just the 20 touched files).
Confirmed each new macro exists in @sasjs/core@5.0.0 (node_modules/@sasjs/core/base/mp_execute.sas, base/mp_ds2ddl.sas, ddl/mddl_dc_locktable.sas, ddl/mddl_dc_difftable.sas).
Verified every call site's signature against the v5 macro definitions — all keyword/positional args are valid:
Confirmed package-lock.json integrity hash and version (5.0.0) are consistent with package.json.
Ran sasjs c -t server-ci — compiles cleanly; coverage figures posted in a separate comment.
Critical
None.
Warnings
None. The two showlog=NO additions (see inline comments) are intentional behavior changes — DDL is no longer echoed to the SAS log on export. This is an improvement (less log noise / less data leakage to logs), but consumers who previously scraped the log for DDL will need to read the streamed file instead.
Suggestions
testinit.sas:13-16 — The @li mp_execute.sas header entry lists a macro that is not actually called in this file, and the pre-existing REMOVE THAT LAST MACRO note still applies. Consider dropping both the @li mp_execute.sas line and the stray comment to resolve this long-standing TODO.
exportdb.sas has no test — services/admin/exportdb.sas shows as not covered in the coverage report (no exportdb.test.sas exists). The sibling getddl.sasis covered by getddl.test.sas. The macro rename in this PR is mechanical, but the admin export path (including the mpeadmins authorization gate) remains untested. Adding a test would be valuable as a follow-up.
Looks Good
The mp_coretable → mddl_dc_* mapping is correct: LOCKTABLE→mddl_dc_locktable, DIFFTABLE→mddl_dc_difftable, and the libds= keyword arg is preserved.
The library-level DDL call in exportdb.sas correctly omits the optional ds positional, matching mp_ds2ddl's library-mode behaviour.
applydttm=YES is preserved on both DDL call sites.
Test files consistently use mp_execute with the same viyacontext=/inputfiles=/inputdatasets=/outlib= parameters the old mp_testservice accepted (it's a thin wrapper over mx_testservice).
Reviewed by Hermes Agent
## Hermes Agent Code Review
**Verdict: Approve** (with minor suggestions)
This PR bumps `@sasjs/core` from 4.68.3 → 5.0.0 (a breaking change) and migrates three renamed/removed macros across the codebase:
- `mp_testservice` → `mp_execute`
- `mp_getddl` → `mp_ds2ddl`
- `mp_coretable(LOCKTABLE|DIFFTABLE,...)` → `mddl_dc_locktable` / `mddl_dc_difftable`
### Verification performed
- Confirmed all three old macro names have **zero** remaining references across the entire repo (not just the 20 touched files).
- Confirmed each new macro exists in `@sasjs/core@5.0.0` (`node_modules/@sasjs/core/base/mp_execute.sas`, `base/mp_ds2ddl.sas`, `ddl/mddl_dc_locktable.sas`, `ddl/mddl_dc_difftable.sas`).
- Verified every call site's signature against the v5 macro definitions — all keyword/positional args are valid:
- `mp_execute(program, inputfiles=, inputdatasets=, outlib=, viyacontext=, mdebug=)` ✓
- `mp_ds2ddl(libref, ds, fref=, flavour=, showlog=, schema=, applydttm=)` ✓
- `mddl_dc_locktable(libds=)` / `mddl_dc_difftable(libds=)` ✓
- Confirmed `package-lock.json` integrity hash and version (5.0.0) are consistent with `package.json`.
- Ran `sasjs c -t server-ci` — compiles cleanly; coverage figures posted in a separate comment.
### Critical
None.
### Warnings
None. The two `showlog=NO` additions (see inline comments) are intentional behavior changes — DDL is no longer echoed to the SAS log on export. This is an improvement (less log noise / less data leakage to logs), but consumers who previously scraped the log for DDL will need to read the streamed file instead.
### Suggestions
1. **`testinit.sas:13-16`** — The `@li mp_execute.sas` header entry lists a macro that is not actually called in this file, and the pre-existing `REMOVE THAT LAST MACRO` note still applies. Consider dropping both the `@li mp_execute.sas` line and the stray comment to resolve this long-standing TODO.
2. **`exportdb.sas` has no test** — `services/admin/exportdb.sas` shows as `not covered` in the coverage report (no `exportdb.test.sas` exists). The sibling `getddl.sas` *is* covered by `getddl.test.sas`. The macro rename in this PR is mechanical, but the admin export path (including the `mpeadmins` authorization gate) remains untested. Adding a test would be valuable as a follow-up.
### Looks Good
- The `mp_coretable` → `mddl_dc_*` mapping is correct: `LOCKTABLE`→`mddl_dc_locktable`, `DIFFTABLE`→`mddl_dc_difftable`, and the `libds=` keyword arg is preserved.
- The library-level DDL call in `exportdb.sas` correctly omits the optional `ds` positional, matching `mp_ds2ddl`'s library-mode behaviour.
- `applydttm=YES` is preserved on both DDL call sites.
- Test files consistently use `mp_execute` with the same `viyacontext=`/`inputfiles=`/`inputdatasets=`/`outlib=` parameters the old `mp_testservice` accepted (it's a thin wrapper over `mx_testservice`).
Reviewed by Hermes Agent
Behavior change: showlog=NO is new here (the old mp_getddl call did not pass it, so DDL was echoed to the log by default). This is a reasonable improvement — the admin export should not dump generated DDL into the SAS log — but it is a semantic change beyond a pure rename. Worth noting in the PR description if not already. The call signature otherwise matches mp_ds2ddl(libref, ds, fref=, flavour=, showlog=, schema=, applydttm=) correctly (library-level mode, ds intentionally omitted).
Behavior change: `showlog=NO` is **new** here (the old `mp_getddl` call did not pass it, so DDL was echoed to the log by default). This is a reasonable improvement — the admin export should not dump generated DDL into the SAS log — but it is a semantic change beyond a pure rename. Worth noting in the PR description if not already. The call signature otherwise matches `mp_ds2ddl(libref, ds, fref=, flavour=, showlog=, schema=, applydttm=)` correctly (library-level mode, `ds` intentionally omitted).
Same as exportdb: showlog=NO is newly added vs the old mp_getddl(..., applydttm=YES) call, so the public DDL-download service no longer writes DDL to the log. Intentional and fine; just flagging it's more than a mechanical rename. Positional args (&libref,&ds) and keyword args (fref=tmp, applydttm=YES) all map correctly onto mp_ds2ddl's signature.
Same as exportdb: `showlog=NO` is newly added vs the old `mp_getddl(..., applydttm=YES)` call, so the public DDL-download service no longer writes DDL to the log. Intentional and fine; just flagging it's more than a mechanical rename. Positional args (`&libref`,`&ds`) and keyword args (`fref=tmp`, `applydttm=YES`) all map correctly onto `mp_ds2ddl`'s signature.
Pre-existing TODO carried forward: REMOVE THAT LAST MACRO refers to the @li mp_execute.sas entry on line 13, which lists a macro that is not called anywhere in this file (only mp_init, mpeinit2, dc_getsettings are invoked). Consider removing both the @li mp_execute.sas line and this stray comment to close out the TODO.
Pre-existing TODO carried forward: `REMOVE THAT LAST MACRO` refers to the `@li mp_execute.sas` entry on line 13, which lists a macro that is **not called** anywhere in this file (only `mp_init`, `mpeinit2`, `dc_getsettings` are invoked). Consider removing both the `@li mp_execute.sas` line and this stray comment to close out the TODO.
Coverage measured via npx @sasjs/cli c -t server-ci (static dependency coverage — which test files reference which source files) from the sas/sasjs/ directory on PR branch corebump (@sasjs/core@5.0.0).
Scope
Covered
Total
%
Services
20
60
33%
Macros
12
42
29%
Overall
32
102
31%
Notes on files touched by this PR
services/public/getddl.sas — covered by services/public/getddl.test.sas. ✅
services/admin/exportdb.sas — not covered. No exportdb.test.sas exists. This is a pre-existing gap (the PR only renamed mp_getddl→mp_ds2ddl inside it); the admin export path and its mpeadmins authorization gate remain untested. Recommended follow-up.
The 16 changed *.test.sas files are test files themselves; their mp_testservice→mp_execute rename compiles cleanly and preserves all viyacontext= / inputfiles= / inputdatasets= / outlib= parameters.
tests/testsetup.sas / tests/testinit.sas — these are the test harness setup; they compile cleanly with the new mddl_dc_locktable / mddl_dc_difftable and mp_execute macros.
Caveats
These are static coverage numbers (file-level reference coverage), not runtime line/branch coverage. Runtime SAS coverage requires an active SAS/Viya server connection.
The compile step succeeded; the only error in the run was an unrelated missing client/dist web bundle (streamConfig.webSourcePath), which has no bearing on the SAS coverage figures above.
Generated by Hermes Agent
## Test Coverage Report
Coverage measured via `npx @sasjs/cli c -t server-ci` (static dependency coverage — which test files reference which source files) from the `sas/sasjs/` directory on PR branch `corebump` (`@sasjs/core@5.0.0`).
| Scope | Covered | Total | % |
|------------|---------|-------|----|
| Services | 20 | 60 | 33% |
| Macros | 12 | 42 | 29% |
| **Overall**| **32** | **102** | **31%** |
### Notes on files touched by this PR
- **`services/public/getddl.sas`** — **covered** by `services/public/getddl.test.sas`. ✅
- **`services/admin/exportdb.sas`** — **not covered**. No `exportdb.test.sas` exists. This is a pre-existing gap (the PR only renamed `mp_getddl`→`mp_ds2ddl` inside it); the admin export path and its `mpeadmins` authorization gate remain untested. Recommended follow-up.
- The 16 changed `*.test.sas` files are test files themselves; their `mp_testservice`→`mp_execute` rename compiles cleanly and preserves all `viyacontext=` / `inputfiles=` / `inputdatasets=` / `outlib=` parameters.
- `tests/testsetup.sas` / `tests/testinit.sas` — these are the test harness setup; they compile cleanly with the new `mddl_dc_locktable` / `mddl_dc_difftable` and `mp_execute` macros.
### Caveats
- These are **static** coverage numbers (file-level reference coverage), not runtime line/branch coverage. Runtime SAS coverage requires an active SAS/Viya server connection.
- The compile step succeeded; the only error in the run was an unrelated missing `client/dist` web bundle (`streamConfig.webSourcePath`), which has no bearing on the SAS coverage figures above.
Generated by Hermes Agent
Coverage measured via npx @sasjs/cli c -t server-ci against the server-ci target.
Category
Covered
Total
Percentage
Services
22
60
37%
Macros
12
42
29%
Overall
34
102
33%
The compile step completed successfully (server-ci target). A non-fatal warning was emitted about webSourcePath: client/dist not existing — expected in this SAS-only checkout context and does not affect coverage measurement.
Generated by Hermes Agent
## Test Coverage Report
Coverage measured via `npx @sasjs/cli c -t server-ci` against the `server-ci` target.
| Category | Covered | Total | Percentage |
|----------|---------|-------|------------|
| Services | 22 | 60 | 37% |
| Macros | 12 | 42 | 29% |
| **Overall** | **34** | **102** | **33%** |
The compile step completed successfully (`server-ci` target). A non-fatal warning was emitted about `webSourcePath: client/dist` not existing — expected in this SAS-only checkout context and does not affect coverage measurement.
Generated by Hermes Agent
This commit (023c29f — "fix: mp_execute dep") addresses prior review feedback cleanly: it adds the two missing admin service tests, fixes a stale doc header, and removes leftover scratch text from testinit.sas.
Critical
None.
Warnings
None.
Suggestions
exportdb.test.sas — assertion granularity
The new test checks the DDL output containsCREATE TABLE / INSERT INTO substrings. For a breaking-change bump of @sasjs/core (4.68.3 → 5.0.0, where mp_getddl was renamed to mp_ds2ddl), this is a reasonable smoke test, but a regression that returns truncated or malformed DDL would still pass as long as the keyword appears anywhere. Asserting against a known table name (e.g. &dc_libref..MPE_TABLES) would make the test more robust against partial output.
refreshcatalog.test.sas — hardcoded libref DCTEST
Test 2 asserts DCTEST tables were catalogued. This couples the test to a specific test-library name that must exist in the server-ci environment. If that library is ever renamed, the test fails for an environmental reason rather than a code regression. Consider deriving the libref from a macro variable if one is available in the test harness.
Looks Good
mpe_getversions.test.sas: Correctly adds mx_testservice.sas and mf_getuniquefileref.sas to the @li doc header — the file already calledmx_testservice but never documented it. Good doc hygiene fix.
testinit.sas: The removed "REMOVE THAT LAST MACRO" stray comment and the dangling mp_execute.sas@li entry (left over from the previous commit) are correctly cleaned up. mp_execute remains documented in testsetup.sas, which is where it's actually invoked.
The v4→v5 migration is complete: a repo-wide grep confirms zero remaining references to mp_getddl, mp_testservice, or mp_coretable outside node_modules.
Reviewed by Hermes Agent
## Hermes Agent Code Review
**Verdict: Approve**
This commit (`023c29f` — "fix: mp_execute dep") addresses prior review feedback cleanly: it adds the two missing admin service tests, fixes a stale doc header, and removes leftover scratch text from `testinit.sas`.
### Critical
None.
### Warnings
None.
### Suggestions
**exportdb.test.sas — assertion granularity**
The new test checks the DDL output *contains* `CREATE TABLE` / `INSERT INTO` substrings. For a breaking-change bump of `@sasjs/core` (4.68.3 → 5.0.0, where `mp_getddl` was renamed to `mp_ds2ddl`), this is a reasonable smoke test, but a regression that returns truncated or malformed DDL would still pass as long as the keyword appears anywhere. Asserting against a known table name (e.g. `&dc_libref..MPE_TABLES`) would make the test more robust against partial output.
**refreshcatalog.test.sas — hardcoded libref `DCTEST`**
Test 2 asserts `DCTEST` tables were catalogued. This couples the test to a specific test-library name that must exist in the `server-ci` environment. If that library is ever renamed, the test fails for an environmental reason rather than a code regression. Consider deriving the libref from a macro variable if one is available in the test harness.
### Looks Good
- `mpe_getversions.test.sas`: Correctly adds `mx_testservice.sas` and `mf_getuniquefileref.sas` to the `@li` doc header — the file already *called* `mx_testservice` but never documented it. Good doc hygiene fix.
- `testinit.sas`: The removed "REMOVE THAT LAST MACRO" stray comment and the dangling `mp_execute.sas` `@li` entry (left over from the previous commit) are correctly cleaned up. `mp_execute` remains documented in `testsetup.sas`, which is where it's actually invoked.
- The v4→v5 migration is complete: a repo-wide grep confirms zero remaining references to `mp_getddl`, `mp_testservice`, or `mp_coretable` outside `node_modules`.
Reviewed by Hermes Agent
The test asserts the DDL containsCREATE TABLE / INSERT INTO but does not verify the DDL is well-formed or targets the correct schema. For a breaking-change bump of @sasjs/core (where mp_getddl was renamed to mp_ds2ddl), a regression returning truncated/malformed DDL would still pass as long as the keyword appears anywhere. Consider asserting against a known table name (e.g. &dc_libref..MPE_TABLES) to make the test more robust against partial output.
The test asserts the DDL *contains* `CREATE TABLE` / `INSERT INTO` but does not verify the DDL is well-formed or targets the correct schema. For a breaking-change bump of `@sasjs/core` (where `mp_getddl` was renamed to `mp_ds2ddl`), a regression returning truncated/malformed DDL would still pass as long as the keyword appears anywhere. Consider asserting against a known table name (e.g. `&dc_libref..MPE_TABLES`) to make the test more robust against partial output.
Hardcoded DCTEST libref couples the test to a specific library that must exist in the server-ci environment. If that library is renamed, the test fails for an environmental reason rather than a code regression. Consider deriving the libref from a macro variable if one is available in the test harness.
Hardcoded `DCTEST` libref couples the test to a specific library that must exist in the `server-ci` environment. If that library is renamed, the test fails for an environmental reason rather than a code regression. Consider deriving the libref from a macro variable if one is available in the test harness.
allan marked this conversation as resolved
allan
merged commit 9b2e0df2b0 into version7-132026-08-14 11:05:19 +00:00
allan
deleted branch corebump2026-08-14 11:05:19 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
hermes
@sasjs/core is bumped to 5.0.0 in sas/package.json and all v5 breaking changes are addressed:
Breaking changes from the v5.0.0 release notes (https://github.com/sasjs/core/releases/tag/v5.0.0) and how dc was affected:
mp_testservice.sas → mp_execute.sas (rename, same parameters) — updated 16 test files under sas/sasjs/** (all the *.test.sas files, tests/testsetup.sas, tests/testinit.sas, and their doxygen @li references).
mp_getddl.sas → mp_ds2ddl.sas — updated:
In v5 SHOWLOG now defaults to YES (was NO), so I added showlog=NO to both calls to preserve the previous behaviour (both append DDL to a fileref and stream it back; log noise isn't wanted).
mp_coretable.sas removed — sas/sasjs/tests/testsetup.sas now calls the standalone DDL macros directly:
%mddl_dc_locktable(libds=...) and %mddl_dc_difftable(libds=...) (exact equivalents, per the release notes).
Checked and unaffected: mp_abort type= removal (dc never passes type=), mf_getuniquelibref maxtries removal (not used), mcf_* insert_cmplib removal / wrap= default change (dc only uses
%mcf_getfmttype(wrap=YES), still valid).
Verification:
(mp_execute, mddl_dc_locktable, mp_ds2ddl) — no dangling references to the removed ones.
Hermes Agent Code Review
Verdict: Approve (with minor suggestions)
This PR bumps
@sasjs/corefrom 4.68.3 → 5.0.0 (a breaking change) and migrates three renamed/removed macros across the codebase:mp_testservice→mp_executemp_getddl→mp_ds2ddlmp_coretable(LOCKTABLE|DIFFTABLE,...)→mddl_dc_locktable/mddl_dc_difftableVerification performed
@sasjs/core@5.0.0(node_modules/@sasjs/core/base/mp_execute.sas,base/mp_ds2ddl.sas,ddl/mddl_dc_locktable.sas,ddl/mddl_dc_difftable.sas).mp_execute(program, inputfiles=, inputdatasets=, outlib=, viyacontext=, mdebug=)✓mp_ds2ddl(libref, ds, fref=, flavour=, showlog=, schema=, applydttm=)✓mddl_dc_locktable(libds=)/mddl_dc_difftable(libds=)✓package-lock.jsonintegrity hash and version (5.0.0) are consistent withpackage.json.sasjs c -t server-ci— compiles cleanly; coverage figures posted in a separate comment.Critical
None.
Warnings
None. The two
showlog=NOadditions (see inline comments) are intentional behavior changes — DDL is no longer echoed to the SAS log on export. This is an improvement (less log noise / less data leakage to logs), but consumers who previously scraped the log for DDL will need to read the streamed file instead.Suggestions
testinit.sas:13-16— The@li mp_execute.sasheader entry lists a macro that is not actually called in this file, and the pre-existingREMOVE THAT LAST MACROnote still applies. Consider dropping both the@li mp_execute.sasline and the stray comment to resolve this long-standing TODO.exportdb.sashas no test —services/admin/exportdb.sasshows asnot coveredin the coverage report (noexportdb.test.sasexists). The siblinggetddl.sasis covered bygetddl.test.sas. The macro rename in this PR is mechanical, but the admin export path (including thempeadminsauthorization gate) remains untested. Adding a test would be valuable as a follow-up.Looks Good
mp_coretable→mddl_dc_*mapping is correct:LOCKTABLE→mddl_dc_locktable,DIFFTABLE→mddl_dc_difftable, and thelibds=keyword arg is preserved.exportdb.sascorrectly omits the optionaldspositional, matchingmp_ds2ddl's library-mode behaviour.applydttm=YESis preserved on both DDL call sites.mp_executewith the sameviyacontext=/inputfiles=/inputdatasets=/outlib=parameters the oldmp_testserviceaccepted (it's a thin wrapper overmx_testservice).Reviewed by Hermes Agent
Behavior change:
showlog=NOis new here (the oldmp_getddlcall did not pass it, so DDL was echoed to the log by default). This is a reasonable improvement — the admin export should not dump generated DDL into the SAS log — but it is a semantic change beyond a pure rename. Worth noting in the PR description if not already. The call signature otherwise matchesmp_ds2ddl(libref, ds, fref=, flavour=, showlog=, schema=, applydttm=)correctly (library-level mode,dsintentionally omitted).Same as exportdb:
showlog=NOis newly added vs the oldmp_getddl(..., applydttm=YES)call, so the public DDL-download service no longer writes DDL to the log. Intentional and fine; just flagging it's more than a mechanical rename. Positional args (&libref,&ds) and keyword args (fref=tmp,applydttm=YES) all map correctly ontomp_ds2ddl's signature.Pre-existing TODO carried forward:
REMOVE THAT LAST MACROrefers to the@li mp_execute.sasentry on line 13, which lists a macro that is not called anywhere in this file (onlymp_init,mpeinit2,dc_getsettingsare invoked). Consider removing both the@li mp_execute.sasline and this stray comment to close out the TODO.Test Coverage Report
Coverage measured via
npx @sasjs/cli c -t server-ci(static dependency coverage — which test files reference which source files) from thesas/sasjs/directory on PR branchcorebump(@sasjs/core@5.0.0).Notes on files touched by this PR
services/public/getddl.sas— covered byservices/public/getddl.test.sas. ✅services/admin/exportdb.sas— not covered. Noexportdb.test.sasexists. This is a pre-existing gap (the PR only renamedmp_getddl→mp_ds2ddlinside it); the admin export path and itsmpeadminsauthorization gate remain untested. Recommended follow-up.*.test.sasfiles are test files themselves; theirmp_testservice→mp_executerename compiles cleanly and preserves allviyacontext=/inputfiles=/inputdatasets=/outlib=parameters.tests/testsetup.sas/tests/testinit.sas— these are the test harness setup; they compile cleanly with the newmddl_dc_locktable/mddl_dc_difftableandmp_executemacros.Caveats
client/distweb bundle (streamConfig.webSourcePath), which has no bearing on the SAS coverage figures above.Generated by Hermes Agent
Test Coverage Report
Coverage measured via
npx @sasjs/cli c -t server-ciagainst theserver-citarget.The compile step completed successfully (
server-citarget). A non-fatal warning was emitted aboutwebSourcePath: client/distnot existing — expected in this SAS-only checkout context and does not affect coverage measurement.Generated by Hermes Agent
Hermes Agent Code Review
Verdict: Approve
This commit (
023c29f— "fix: mp_execute dep") addresses prior review feedback cleanly: it adds the two missing admin service tests, fixes a stale doc header, and removes leftover scratch text fromtestinit.sas.Critical
None.
Warnings
None.
Suggestions
exportdb.test.sas — assertion granularity
The new test checks the DDL output contains
CREATE TABLE/INSERT INTOsubstrings. For a breaking-change bump of@sasjs/core(4.68.3 → 5.0.0, wheremp_getddlwas renamed tomp_ds2ddl), this is a reasonable smoke test, but a regression that returns truncated or malformed DDL would still pass as long as the keyword appears anywhere. Asserting against a known table name (e.g.&dc_libref..MPE_TABLES) would make the test more robust against partial output.refreshcatalog.test.sas — hardcoded libref
DCTESTTest 2 asserts
DCTESTtables were catalogued. This couples the test to a specific test-library name that must exist in theserver-cienvironment. If that library is ever renamed, the test fails for an environmental reason rather than a code regression. Consider deriving the libref from a macro variable if one is available in the test harness.Looks Good
mpe_getversions.test.sas: Correctly addsmx_testservice.sasandmf_getuniquefileref.sasto the@lidoc header — the file already calledmx_testservicebut never documented it. Good doc hygiene fix.testinit.sas: The removed "REMOVE THAT LAST MACRO" stray comment and the danglingmp_execute.sas@lientry (left over from the previous commit) are correctly cleaned up.mp_executeremains documented intestsetup.sas, which is where it's actually invoked.mp_getddl,mp_testservice, ormp_coretableoutsidenode_modules.Reviewed by Hermes Agent
The test asserts the DDL contains
CREATE TABLE/INSERT INTObut does not verify the DDL is well-formed or targets the correct schema. For a breaking-change bump of@sasjs/core(wheremp_getddlwas renamed tomp_ds2ddl), a regression returning truncated/malformed DDL would still pass as long as the keyword appears anywhere. Consider asserting against a known table name (e.g.&dc_libref..MPE_TABLES) to make the test more robust against partial output.Hardcoded
DCTESTlibref couples the test to a specific library that must exist in theserver-cienvironment. If that library is renamed, the test fails for an environmental reason rather than a code regression. Consider deriving the libref from a macro variable if one is available in the test harness.