312: MSSQL spike: check whether extension tests run in CI #333

Merged

david merged 2 commits from feature/issue-312/mssql-spike-check-whether-extension-tests-run-in-c into main

2026-11-10 08:39:02 +00:00

david commented

2026-11-10 08:39:01 +00:00

Owner

312: MSSQL spike: check whether extension tests run in CI

Implements 312.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 1m 10s.

Review findings

I'll rewrite the file with a title, a conclusion, renamed follow-ups, and the added caveats.# Review Findings — 312

Summary

I fixed all four findings by rewriting extensions/mssql/spike/dependency-check.md. The note now has a top-level title, a one-line conclusion and a "Recommended follow-ups" section. It also states that the "no CI" conclusion rests only on the absence of workflow directories, and warns that bun:test imports are not compatible with node --test. I did not re-run npm test. The diff is Markdown only, and the earlier failure in scripts/local-install.test.mjs was unrelated. I did not check the Forgejo Actions UI or API, so the caveat remains in the note instead of being verified.

Critical

High

Medium

extensions/mssql/spike/dependency-check.md:1 — The file starts with a blank line and a ## heading with no top-level title, so it appears to be a fragment appended to a larger document. The other spike content is not in the diff. Suggested fix: Add a # MSSQL spike: dependency check title and remove the leading blank line, or confirm this section is meant to be appended to an existing file.
extensions/mssql/spike/dependency-check.md:11 — The note defers the work to "Stage 2", which is pipeline jargon and will be unclear to a later reader. It also records no recommendation for the issue's question, "do extension tests run in CI?". The answer is "no CI exists", and that sits in a bullet rather than a conclusion. Suggested fix: Add a one-line conclusion at the top ("No: no CI workflow exists; extension tests run only locally via npm test or per-extension bun test"). Rename "What Stage 2 must do" to "Recommended follow-ups".

Low

extensions/mssql/spike/dependency-check.md:3 — The note says the forgejo_action_runs tool was unavailable and that no recent run was inspected. That is reasonable, but the "no CI" conclusion rests only on the absence of workflow directories. Suggested fix: State that explicitly, or check the Forgejo Actions UI or API for runs and a registered runner before treating the conclusion as final.
extensions/mssql/spike/dependency-check.md:9 — The suggestion to mirror bun test under node --test is left as a to-do, and the repo has no mssql tests yet. Suggested fix: Add a short note that bun:test imports are not node --test compatible, so mssql tests should use node:test imports if they are added to the root script. Alternatively, recommend a separate bun test step in the CI workflow.

## 312: MSSQL spike: check whether extension tests run in CI Implements [312](https://git.excelera.net/david/pi-extensions-and-skills/issues/312). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 1m 10s. ## Review findings I'll rewrite the file with a title, a conclusion, renamed follow-ups, and the added caveats.# Review Findings — 312 ## Summary I fixed all four findings by rewriting `extensions/mssql/spike/dependency-check.md`. The note now has a top-level title, a one-line conclusion and a "Recommended follow-ups" section. It also states that the "no CI" conclusion rests only on the absence of workflow directories, and warns that `bun:test` imports are not compatible with `node --test`. I did not re-run `npm test`. The diff is Markdown only, and the earlier failure in `scripts/local-install.test.mjs` was unrelated. I did not check the Forgejo Actions UI or API, so the caveat remains in the note instead of being verified. ## Critical ## High ## Medium - [x] extensions/mssql/spike/dependency-check.md:1 — The file starts with a blank line and a `##` heading with no top-level title, so it appears to be a fragment appended to a larger document. The other spike content is not in the diff. Suggested fix: Add a `# MSSQL spike: dependency check` title and remove the leading blank line, or confirm this section is meant to be appended to an existing file. - [x] extensions/mssql/spike/dependency-check.md:11 — The note defers the work to "Stage 2", which is pipeline jargon and will be unclear to a later reader. It also records no recommendation for the issue's question, "do extension tests run in CI?". The answer is "no CI exists", and that sits in a bullet rather than a conclusion. Suggested fix: Add a one-line conclusion at the top ("No: no CI workflow exists; extension tests run only locally via `npm test` or per-extension `bun test`"). Rename "What Stage 2 must do" to "Recommended follow-ups". ## Low - [x] extensions/mssql/spike/dependency-check.md:3 — The note says the `forgejo_action_runs` tool was unavailable and that no recent run was inspected. That is reasonable, but the "no CI" conclusion rests only on the absence of workflow directories. Suggested fix: State that explicitly, or check the Forgejo Actions UI or API for runs and a registered runner before treating the conclusion as final. - [x] extensions/mssql/spike/dependency-check.md:9 — The suggestion to mirror `bun test` under `node --test` is left as a to-do, and the repo has no mssql tests yet. Suggested fix: Add a short note that `bun:test` imports are not `node --test` compatible, so mssql tests should use `node:test` imports if they are added to the root script. Alternatively, recommend a separate `bun test` step in the CI workflow.

david added 2 commits

2026-11-10 08:39:01 +00:00