ci: run every suite on PRs and main - #19
Merged
Merged
Conversation
No CI existed: four test suites (Node, PHP, mobile, web) and nothing ran them, so PRs merged on manual verification alone. - javascript: test/typecheck/build per package, on the declared engines floor and the current LTS - php: the zero-dependency scoring suite, then PHPUnit, on the composer floor and a current release - timezones: scoring under non-UTC PHP timezones, guarding decisions.md #29 - ci: aggregation gate, so branch protection has one stable check name Hardening: read-only token, credentials not persisted to .git/config, and third-party actions pinned to a commit SHA rather than a mutable tag.
First CI run failed all three Node 18.14 legs with 'Could not find src/**/*.test.ts'. Node's test runner only gained glob support in 21; on 18 it treats the pattern as a literal path. That's a limit of the test scripts, not of the published packages — consumers install built output and never invoke them — so the floor keeps typecheck and build, which is what actually shows the package compiles for the version engines promises. Making the suites run there would mean adding a glob dependency for an already-EOL runtime.
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.
There was no CI. Four test suites existed (
referral-web,referral-mobile,referral-sdk-node,referral-sdk) and nothing ran them, so PRs merged on manual verification alone — including #16, which shipped with zero status checks.Jobs
javascriptphptimezonesciHardening
permissions: contents: read— bothnpm ciandcomposer installrun dependency lifecycle scripts, so a compromised transitive package must not get a token that can write to the repopersist-credentials: false—actions/checkoutotherwise leaves the token in.git/configwhere any build script can read itshivammathur/setup-phppinned to commitf3e473d(v2.37.2) rather than the mutablev2tag.actions/*are GitHub-owned, where tag pinning is accepted practiceNotes on a few choices
Version floors are intentionally EOL. Node 18.14 and PHP 8.1 are what
enginesandcomposer.jsonpromise, so they stay tested; a declared floor doesn't make anyone secure or insecure, and dropping it wouldn't patch a user's runtime, only stop them installing. Each matrix also tests a current version, which is what actually catches breaks.Concurrency cancels PR runs only. Cancelling on
mainwould leave default-branch commits with no completed CI result, which breaks bisecting after a fast series of merges.Composer cache keys on
composer.json, notcomposer.lock— this is a library and deliberately ships no lockfile, so hashing the lock would resolve empty and the key would never invalidate.The timezone job is one job, not a matrix. The suite is ~0.05s of work, so four runner VMs cost far more than they return. The loop runs every zone and reports all of them rather than aborting on the first failure — the pattern of which zones fail is what identifies an offset bug. It uses
-d date.timezone, notTZ: PHP's date functions read the INI setting and ignore the environment variable (verified —TZalone leavesdate_default_timezone_get()at UTC andstrtotime()unchanged). Offsets include 45-minute ones.Verification
Every job simulated locally against post-#18
main: all three packages green on test/typecheck/build, and the scoring suite 14/14 under Africa/Lagos, America/New_York, Asia/Kolkata and Pacific/Chatham. Rebased ontomainafter #18 so the first run should be green rather than red-on-arrival.Follow-up worth considering once this lands: require the
cicheck in branch protection.