fix: make the SQL integration suite runnable, and honest when it is not - #11
Merged
Merged
Conversation
Fixing the path in #9 only got the file found. Actually running it surfaced two things. The describe-level gate is `URL_ ? describe : describe.skip`, which tests whether RESILIX_TEST_DATABASE_URL is SET — but `pnpm test:integration` always sets it, defaulting to localhost:5459. So the script can never distinguish "asked for integration tests" from "has a database", and without one the failure was a bare AggregateError with two node:net frames and no indication of what to do. It now names the URL it tried, carries the docker run line, and points at `pnpm test` to skip. Then it was run for real, against PostgreSQL 14.15, which had not happened since the reorg: 13 of 13 pass. That is a second data point for classifySql — the mappings were captured on PostgreSQL 16 and hold on 14 too. One trap found while setting that up, now in CONTRIBUTING: initdb --auth=trust makes the "bad password -> transient" case unfalsifiable, because a bad password does not fail. It reports `NO THROW` until pg_hba.conf uses scram-sha-256 for 127.0.0.1. Twelve of thirteen passing with the thirteenth failing for a harness reason is exactly the shape that gets mistaken for a library bug.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 36194417 | Triggered | Generic Password | 6cfe381 | src/scenarios/sql-integration.test.ts | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Adding test:compat to CI in #9 turned an environment-sensitive check into a required one, and it failed on the first PR after: 170 of 362, with test.js reporting STALLED in CI while passing locally in 6 seconds. Two problems behind that, both worth fixing regardless of which one it was. The suite tracked opossum's main branch. .opossum-compat/ is not cached in CI, so every run fetched afresh and the README's "362 of 362" was being measured against whatever opossum had merged that morning. The expected value of a required check must not be something upstream can change without us. It is now pinned to decbedf6, and bumping it is a deliberate act. And a stall reported only the word STALLED, discarding the child's stdout, stderr and exit status. That is why the CI failure could not be diagnosed and had to be guessed at — first as a 60s timeout, which the 16-second total run already ruled out. It now prints the exit code, the signal, and the last twelve lines the child produced, or says explicitly that there was none. 362 of 362 locally at the pin. Whether CI agrees is now something the output will explain rather than something to infer.
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
1 participant
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.
#9 fixed the path so the file is found. Running it revealed the rest.
RESILIX_TEST_DATABASE_URLis set, but the script always sets it with a localhost default — so without a database you got a bareAggregateErrorand twonode:netframes. Now it names the URL, gives thedocker runline, and points atpnpm test.classifySql's mappings were captured on 16 and hold on 14.initdb --auth=trustmakes "bad password" unfalsifiable, so that test reportsNO THROWuntilpg_hba.confusesscram-sha-256.