Run test suite in CI; fix missing @types/node dependency - #18
Merged
Conversation
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.
What changed
CI never ran the test suite.
.github/workflows/ci.ymlalready ran typecheck, lint, and build on every push/PR, but had
no step running
npm test— meaning the 21 unit tests added in aprior PR have never actually executed in CI. Added a "Run tests"
step (
npm test) between lint and build.@types/nodewas missing from devDependencies.src/lib/stellar.tsuses
Bufferandrequire, but the types this depends on wereonly present via transitive hoisting from another package — not
pinned anywhere. That makes the DTS build non-reproducible: a
different dependency resolution (e.g. in a clean CI runner) could
silently fail. Added it explicitly.
Known follow-up (not fixed here, flagged for visibility)
The
lintjob in this same workflow fails on every run regardlessof this PR — there's no ESLint config file in the repo at all,
despite
eslintbeing installed and alintscript defined.Running
npm run lintlocally confirms this:"ESLint couldn't find a configuration file."
Happy to follow up with a dedicated PR adding a basic ESLint config
if useful — didn't want to bundle an opinionated rule-set choice
into this PR.
Testing
npx vitest run— 21/21 passingnpm run build— CJS, ESM, DTS all succeed