Repository navigation
feat(compile): write the test coverage report to the build folder - #1506
Conversation
Test coverage is computed on every compile to print the coverage table, but the figures only ever reached the console, so a CI job had to scrape the log to publish them. Write the same information to coverage.json in the build folder, next to the testFlow.json that compile already emits: a summary (covered, total and percentage per type and overall) plus a row per file the table lists. No flag and no new computation - the numbers are the ones already calculated for the table, so the report cannot drift from what the user sees.
Coverage reportTotal coverage
Show files with reduced coverage 🔻Reduced coverage
Report generated by 🧪jest coverage report action from 3e07fb8 |
There was a problem hiding this comment.
Reviewed the full diff at head 8fcdb16 (6 files, +117/-18), the TypeScript hardening checklist, CI at the head sha and the single commit subject feat(compile): .... CI is green at this sha (test (22) x2, coverage annotation). Code review only, no approvals.
-
src/commands/compile/internal/compileTestFile.ts:327 - uncovered macros are keyed by an absolute path.
notCoveredMacrosare rewritten tomacro.replace(macroTypeRegExp, macroFolder + path.sep), andmacroFoldercomes fromgetMacroFolders()which returns paths absolute againstbuildSourceFolder. Socoverage- and nowreport.files- mixes project-relative keys (services/common/appinit.sas) with build-machine absolute keys (/home/ci/app/sasjs/macros/foo.sas). In the console table that was cosmetic; in a published artifact it makes the file machine-dependent (two machines produce different JSON for the same project), leaks the build directory layout into anything that commits or uploadscoverage.json, and breaks a consumer keying on relative paths. Keep the originalmacros/<name>key, or relativize againstprocess.projectDirbefore serialising. The spec fixture has zero macros, so this path is untested - add one macro to the fixture and assert its key is project-relative. -
src/commands/compile/internal/compileTestFile.ts:389 -
filesonly ever contains not-covered files and standalone tests; covered files are never assigned (coverage[file]is written only in thenotCovered*andextraTestsloops). TheCoverageReportJSDoc says "filescarries a row per file the table prints (type and state)", which reads as a per-file coverage map, so a consumer doingfiles['services/common/x.sas']getsundefinedfor a covered service with no way to tell "covered" from "absent". Either emit covered rows under a newCoverageStatevalue, or state explicitly in the type doc and the PR body that the map lists only exceptions.
2 findings above for review.
…red files The report reused the console table's map, which has two properties that are fine on a terminal and wrong in a published artifact: - uncovered macros were keyed by the absolute path the table shows, so two machines produced different JSON for the same project and the build directory layout leaked into anything that committed or uploaded it. - only exceptions were present - a covered service was missing from `files` rather than marked covered, so a consumer could not tell "covered" from "not in this report". Build the report from the project-relative keys `toCover` already holds (`macros/<name>`, like services and jobs) and mark every file `covered` or `not covered`, adding `CoverageState.covered`. The console table is unchanged - it still prints the exceptions only, with absolute macro paths to help a reader find the file.
|
Both findings fixed in feaf635. 1. Machine-dependent keys (compileTestFile.ts:327) - fixed. The report no longer reuses the console table's map: Proof, on a real app with one covered macro and one uncovered macro - table: file: "macros/mymacro.sas": { "Type": "macro", "Coverage": "covered" },
"macros/othermymacro.sas": { "Type": "macro", "Coverage": "not covered" }with 2. Covered files missing from
|
|
CI is green on feaf635: |
The table and coverage.json now come from one map, so the console shows the same keys and rows the report serialises: project-relative paths for macros too, and covered files alongside the exceptions. The absolute-path rewrite that existed only to help a human locate a macro existed nowhere else, so it goes, along with the second map that duplicated the first.
|
Folded the table change in as well, in Rationale: Same app, Every other row and every summary line is identical, and Tests: both specs from the last push now assert the table as well - the uncovered macro is |
…#493) Two CI fixes to the same set of workflows. They are independent of each other and can be reviewed separately; they are together here because both keep the pipeline honest about what it published. ## 1. Unblock the publish job: remove the dead VPN steps ### Why The release job on main has failed on every push since #492: ##[group]Run echo "$CA_CRT" > .github/vpn/ca.crt .github/vpn/ca.crt: No such file or directory ##[error]Process completed with exit code 1 #492 removed `.github/vpn/config.ovpn` (the pipeline no longer starts a VPN - its `openvpn3` calls were already commented out), but the `Write VPN Files` step still writes four cert files into that directory. With the directory gone from the checkout the step fails, and it fails *before* `Semantic Release`, so nothing is published to npm. The merge of #487 is a concrete case: it is on main, `SASjs Core Publish` is red, and `@sasjs/core` is still 6.1.2 on npm - so there is no version for downstream projects to pin. ### What this changes Removes three steps from the release job, all of which are dead: - `Write VPN Files` - writes certs into a directory that no longer exists - `Install Open VPN` - every command commented out - `Start Open VPN 3` - the only command commented out Nothing else in the job reads `.env.server` or the cert files, so the remaining steps are unaffected. ### Verification `yaml.safe_load` parses the workflow, and the release job's step list reads: Checkout, Setup Node.js, Ensure npm supports Trusted Publishing, Install dependencies, Audit dependencies, Check code style, Add credentials, Semantic Release, SAS Packages Release, Debug app token inputs, Generate GitHub App token for docs site, Update Docs Site No `vpn` references remain in the file. Once this is on main the publish workflow reaches Semantic Release and produces the version that carries #487; I will re-run the failed publish for `bd8810d` (or let the next push to main release it) so downstream projects have something to pin. ## 2. Publish compile coverage on the PR as one edited comment The compile job already prints how many services, jobs and macros have a test. `@sasjs/cli` 6.4.0 writes those same figures to `sasjsbuild/coverage.json` (sasjs/cli#1506), so CI now reads the report instead of scraping console output, and publishes it as a PR comment. - **One comment, edited in place.** The body starts with a hidden marker; each run lists the PR's comments, finds the one carrying the marker and edits it. A re-run of the same commit rewrites it, and a new push rewrites it with the new commit named - no second comment, and no stack of them on a long-lived PR. - **No stale figures.** If a run produces no report (a compile that fails before writing one), an existing comment is rewritten to say coverage was not measured on that commit, rather than left showing numbers from an earlier push. With no comment yet, the step posts nothing. - **Report only.** The step runs on `always()` and never fails the job. Coverage here counts macros that have a test, so a low percentage is a work list, not a build error. - **Same-repo PRs only.** `head.repo.full_name == github.repository` skips fork PRs, whose token cannot write to the base repo. Permissions change with it: `checks: write` becomes `pull-requests: write`, and nothing else is granted. The previous commit on this branch published the same numbers as a check run; a comment is where a reviewer looks for them, and it can carry the list of uncovered files, which a check-run summary cannot do as usefully. ### Verification The step's script is pulled out of the YAML and run against the `coverage.json` this tree actually produced (`npx @sasjs/cli@6.4.0 compile -t server`), with the GitHub API stubbed - 22 assertions, all passing: - first run creates one comment, its table matching the report (`124/257 macros, 48%`), with the 133 untested files under a `<details>` block; - a re-run of the same commit edits that comment and creates nothing; - a new push edits the same comment with the new commit named; - no report and no comment: posts nothing, logs why; - no report but a comment from an earlier push: that comment is replaced with the "no coverage report" line and no percentage survives. The 48% is the cross-check: it is the same number the earlier check-run version produced by parsing the log, so the report and the console output agree. The comment this step produces reads: ### SASjs compile coverage Measured by `sasjs compile` on `f4a7c3a`. This comment is edited in place, so it shows the latest run. | Area | Covered | % | | --- | --- | --- | | Services | 0/0 | - | | Jobs | 0/0 | - | | Macros | 124/257 | 48% | | Overall | 124/257 | 48% | <details><summary>133 file(s) with no test</summary> ... </details> `yaml.safe_load` parses the workflow. On this branch's CI run the compile finished, then the step posted the comment above; re-running the workflow on the same commit logs `updated comment 6481238094` and the PR still carries one coverage comment. --------- Co-authored-by: dc <dc@4gl.io> Co-authored-by: sasjs-dev[bot] <sasjs@4gl.io>
Intent
sasjs compilealready computes test coverage - it prints a table of services, jobs and macros plus a per-type summary - but those numbers only exist in the console output of the run. Nothing can consume them: a CI job has to scrape the log, which is brittle and loses the per-file detail.This writes the same information to
coverage.jsonin the build folder, next to thetestFlow.jsonthatcompilealready emits. There is no new flag, because there is no new work: the coverage numbers are computed for the console table on every compile, so the only cost is serialising a small object and writing one small file (a few hundred bytes intosasjsbuild, which is recreated on each compile and already git-ignored).The table and the file are one map rendered twice, so the log a reviewer reads and the artifact a pipeline reads cannot disagree.
Example
sasjs compile -t viyaon an app with two services, one job, one covered macro and one uncovered macro:sasjsbuild/coverage.json:{ "summary": { "services": { "covered": 0, "total": 2, "percentage": 0 }, "jobs": { "covered": 0, "total": 1, "percentage": 0 }, "macros": { "covered": 1, "total": 2, "percentage": 50 }, "overall": { "covered": 1, "total": 5, "percentage": 20 } }, "files": { "services/services/common/appinit.sas": { "Type": "service", "Coverage": "not covered" }, "services/services/common/getdata.sas": { "Type": "service", "Coverage": "not covered" }, "jobs/jobs/daily/load.sas": { "Type": "job", "Coverage": "not covered" }, "macros/mymacro.sas": { "Type": "macro", "Coverage": "covered" }, "macros/othermymacro.sas": { "Type": "macro", "Coverage": "not covered" } } }summarycarries the totals the console prints, as numbers so a badge or a threshold check does not have to parse"25%".filesis a complete per-file map: every service, job and macro the compile found, markedcoveredornot covered, plus standalone tests. A consumer can tell a covered file from one that is not in the project at all.services/services/common/...is the keycompilealready prints whenserviceFoldersis["sasjs/services"]- how the key is derived is untouched by this PR.)Implementation
printTestCoveragebuilds oneCoveragemap - project-relative keys, every file,covered/not covered/standalone- and both renders it as the console table and serialises it asfiles. The table and the report cannot drift.CoverageStategainscovered;CoverageReport/CoverageSummaryare new types insrc/types/testing.ts, andcoverageFileNameis exported fromcompileTestFileso callers and specs do not hardcode the name.testFlow.json(also absent in that case).compileandcompileTestFloware untouched.Checks
npx prettier --check "{src,test}/**/*.{ts,js,html,css,yml,md}"is clean andnpx tsc -p . --noEmitis clean. (README.mdis flagged by that prettier check onmainalready and was left untouched by--write; only one bullet changed.)coveredand listed in the table, and no report when no tests were compiled.src/commands/compile: 9 suites / 104 tests. Full mocked suite: 86 suites / 776 tests. Two suites are skipped locally because the tool they shell out to is missing on this box, not because of this branch:versionrunsnpm list -g @sasjs/cli(no global install here) anddocneeds Doxygen (spawnSync doxygen ENOENT). CI runs both.3e07fb8d: bothtest (22)matrix jobs and the jest coverage report action aresuccessVerification
Built the CLI at
feaf635and at this head, and compiled the same app with both.Before (
feaf635) - the covered macro is missing, and the uncovered one carries the build machine's path:After (this head):
coverage.jsonis byte-identical between the two heads (diffclean), so the table change does not move the artifact.Object.keys(files).filter(path.isAbsolute)is empty, and no cell of the console table is an absolute path.feaf635: running the current spec in afeaf635worktree reportsExpected value: ["macros/coveredmacro.sas", "macro", "covered"]and receives rows that omit it, andExpected value: ["macros/uncoveredmacro.sas", "macro", "not covered"]against a row keyed by the absolute path.sasjs compileon a project with no test files still produces nocoverage.jsonand notestFlow.json.