|
| 1 | +# MySQL2 — Agent Instructions |
| 2 | + |
| 3 | +You are an expert Node.js developer contributing to MySQL2, a high-performance MySQL driver focused on compatibility. |
| 4 | + |
| 5 | +## Project |
| 6 | + |
| 7 | +- Minimum compatibility: **Node 14** (ignore the `engines` field in `package.json`). |
| 8 | +- Core: `/lib` → exposed via `index.js` (callback) and `promise.js` (promise-based). |
| 9 | +- Types: `/typings` → exposed via `index.d.ts` and `promise.d.ts`. |
| 10 | +- Documentation: `website/docs/` (Docusaurus). |
| 11 | +- Tests: |
| 12 | + - Unit: `test/unit` (parallel) |
| 13 | + - Integration: `test/integration` (parallel, runs alongside unit tests) |
| 14 | + - Global: `test/global` (sequential — robust tests that affect MySQL Server global state with advanced cleanup) |
| 15 | + |
| 16 | +--- |
| 17 | + |
| 18 | +## Tests |
| 19 | + |
| 20 | +The test runner is **Poku** ([docs](https://poku.io/docs) · [repo](https://github.com/wellwelwel/poku)). |
| 21 | + |
| 22 | +Poku treats `async`/`await` just like standard JavaScript: use `await` on `describe`/`it`/`test` **only** when the callback is asynchronous. Otherwise, do not include `async` or `await`. |
| 23 | + |
| 24 | +Test files use `.mts` (ESM TypeScript) and support top-level `await`. |
| 25 | + |
| 26 | +Assertions, utilities, and test structure come from Poku: |
| 27 | + |
| 28 | +```ts |
| 29 | +import { describe, it, assert, skip, sleep, strict } from 'poku'; |
| 30 | +``` |
| 31 | + |
| 32 | +- `skip` — Skips the entire test file and reports it (e.g., Deno-only tests, specific Node versions, etc.). |
| 33 | +- `sleep` — Waits for a given duration when needed: `await sleep(100)`. |
| 34 | + |
| 35 | +### `async`/`await` |
| 36 | + |
| 37 | +**Asynchronous:** |
| 38 | + |
| 39 | +```ts |
| 40 | +await describe('test', async () => { |
| 41 | + const connection = await createConnection(); |
| 42 | + |
| 43 | + await it('should do something', async () => { |
| 44 | + const result = await connection.promise().query('SELECT 1'); |
| 45 | + assert(result); |
| 46 | + }); |
| 47 | + |
| 48 | + await connection.promise().end(); |
| 49 | +}); |
| 50 | +``` |
| 51 | + |
| 52 | +**Synchronous:** |
| 53 | + |
| 54 | +```ts |
| 55 | +describe('test', () => { |
| 56 | + it('should do something', () => { |
| 57 | + strict.equal(1 + 1, 2); |
| 58 | + }); |
| 59 | +}); |
| 60 | +``` |
| 61 | + |
| 62 | +### Connection scope and resource cleanup |
| 63 | + |
| 64 | +Never close the connection in the same scope as an assertion that may fail. If the assertion throws, `end()` will never be reached and the process will hang indefinitely. |
| 65 | + |
| 66 | +Separate creation/cleanup into an outer scope: |
| 67 | + |
| 68 | +```ts |
| 69 | +// ❌ Wrong — end() in the same scope as the assertion |
| 70 | +await describe('test', async () => { |
| 71 | + it('should do something', async () => { |
| 72 | + const connection = await createConnection(); |
| 73 | + assert(false); |
| 74 | + await connection.end(); // never reached |
| 75 | + }); |
| 76 | + // process hangs |
| 77 | +}); |
| 78 | + |
| 79 | +// ✅ Correct — end() in an outer scope |
| 80 | +await describe('test', async () => { |
| 81 | + const connection = await createConnection(); |
| 82 | + |
| 83 | + it('should do something', () => { |
| 84 | + assert(false); // fails in its own scope |
| 85 | + }); |
| 86 | + |
| 87 | + await connection.end(); // always reached |
| 88 | +}); |
| 89 | +``` |
| 90 | + |
| 91 | +- Applies to any teardown method (`close`, `end`, `destroy`, `release`) and any connection type (`Connection`, `Pool`, `PoolCluster`, etc.). |
| 92 | +- Use nested or dedicated `describe` blocks to isolate each connection. |
| 93 | +- The same applies to callbacks — `end()` may be inside a nested callback that is never invoked if an assertion fails first. |
| 94 | + |
| 95 | +### Closing connections |
| 96 | + |
| 97 | +Prefer `await conn.promise().end()` instead of wrapping callbacks in `new Promise`: |
| 98 | + |
| 99 | +```ts |
| 100 | +// ❌ Avoid |
| 101 | +await new Promise<void>((resolve) => pool.end(() => resolve())); |
| 102 | + |
| 103 | +// ✅ Prefer |
| 104 | +await pool.promise().end(); |
| 105 | +``` |
| 106 | + |
| 107 | +### Prefer promise-based API |
| 108 | + |
| 109 | +When writing new tests, prefer the promise-based API via `.promise()`. Use callbacks only for testing events, streams, features unavailable in the promise API, or when a feature genuinely needs coverage in both modes (callback + promise). |
| 110 | + |
| 111 | +> Recommendation, not a strict rule. |
| 112 | +
|
| 113 | +```ts |
| 114 | +const connection = createConnection({ |
| 115 | + /* ... */ |
| 116 | +}).promise(); |
| 117 | +const pool = createPool({ |
| 118 | + /* ... */ |
| 119 | +}).promise(); |
| 120 | + |
| 121 | +const cluster = createPoolCluster({ |
| 122 | + /* ... */ |
| 123 | +}); |
| 124 | +cluster.add('node1', { |
| 125 | + /* ... */ |
| 126 | +}); |
| 127 | +const clusterConnection = await cluster.promise().getConnection(); |
| 128 | +``` |
| 129 | + |
| 130 | +### Reference files |
| 131 | + |
| 132 | +| File | Description | |
| 133 | +| ------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------- | |
| 134 | +| `poku.config.js` | Poku config: parallel/sequential suites, timeouts, test directories (`test/unit`, `test/integration`, `test/global`) | |
| 135 | +| `test/common.test.mts` | Shared helpers: `createConnection`, `createPool`, `createPoolCluster`, `getConfig`, `createServer`, `getMysqlVersion`, etc. **Read this before writing new tests.** | |
| 136 | +| `test/docker-compose.yml` | Local environment with MySQL, Node, Deno, Bun, and coverage | |
| 137 | + |
| 138 | +### Useful scripts |
| 139 | + |
| 140 | +```sh |
| 141 | +npm run typecheck # type-check the project |
| 142 | +npm run lint:fix # fix lint and formatting |
| 143 | +FILTER=test/unit/my-test.mts npx poku # run a specific test via Poku |
| 144 | +npx tsx test/unit/my-test.mts # run a test directly |
| 145 | +``` |
| 146 | + |
| 147 | +--- |
| 148 | + |
| 149 | +## Pull Request Reviews |
| 150 | + |
| 151 | +When reviewing a PR, alert the author in the following cases: |
| 152 | + |
| 153 | +- **Fix without tests** — Bug fix without tests covering the fixed scenario. Request tests that would fail without the fix. |
| 154 | +- **Feature without tests** — New feature without test coverage. |
| 155 | +- **Feature without docs** — New feature without documentation. |
| 156 | + |
| 157 | +### Required checklist |
| 158 | + |
| 159 | +Before approving any PR, verify: |
| 160 | + |
| 161 | +**General:** |
| 162 | + |
| 163 | +1. **Tests** — Does the bug fix or feature include tests? |
| 164 | +2. **Documentation** — Do new features include documentation? |
| 165 | + |
| 166 | +**Tests:** |
| 167 | + |
| 168 | +3. **Connection scope** — Is `end()`/`close()`/`destroy()`/`release()` in an outer scope, separate from assertions? |
| 169 | +4. **`process.exit`** — If used to skip a test conditionally, suggest using Poku's `skip` instead. |
| 170 | +5. **`new Promise` + `setTimeout`** — Suggest using Poku's `sleep` instead. |
| 171 | +6. **`node:assert` or `node:test`** — Suggest importing from `poku` instead. Require `strict` instead of `assert`. |
| 172 | + |
| 173 | +Do not approve a PR that violates the items above without first alerting the author. |
0 commit comments