Skip to content

test: explicit test data instead of migration seed rows - #78

Merged
sysid merged 8 commits into
mainfrom
test/explicit-test-data
Sep 30, 2026
Merged

test: explicit test data instead of migration seed rows#78
sysid merged 8 commits into
mainfrom
test/explicit-test-data

Conversation

@sysid

@sysid sysid commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Why

The first migration inserted 14 sample rows (incl. local /Users/... paths) that every test DB inherited implicitly. Tests depended on them without saying so — get_bookmark(1) was Google — and create-db deleted them again right after migrating. One test (action_service shell bookmark) never exercised the bookmark it added and passed anyway.

What

  • Migration: remove the seed INSERT. Existing databases are unaffected (Diesel does not re-run applied migrations; create-db already emptied the table).
  • Tests declare their data: 18 tests that failed without the rows now add their own. 12 more had silently become vacuous on an empty DB (assertions only inside for b in &results or <=/>=); they now assert exact, non-empty result sets.
  • Production cleanup: drop the empty_bookmark_table() workaround in create-db, the method itself, and an unreachable duplicate create_db in cli/bookmark_commands.rs.
  • Datasets as JSON: lsp and hsearch move from bkmr add calls in seed.sh to bkmr/tests/resources/datasets/*.json, loaded via bkmr load-json — one source for cargo tests, scenario scripts and manual editor testing. hsearch descriptions are frozen, so scenarios no longer depend on live web pages.
  • New tests: create-db guards (empty DB; --pre-fill yields exactly the demo entries), the documented LSP completion table asserted over stdio, and the show test now checks displayed entries instead of only the exit code.
  • Docs: "Test data" rule and dataset description in TESTING.md; one line in CONTRIBUTING.md.

Behaviour notes

  • load-json drops unknown system-style tags, so the hsearch tag _procedure_ is now the plain tag procedure (dataset, verify.sh, TESTING.md). Rank-based TC expectations (TC-02/03/04/14) re-checked: unchanged.
  • verify.sh tag-filter and limit checks now require exact counts, so an empty result can no longer pass.
  • TESTING.md corrections of pre-existing inaccuracies: hsearch group E is embedded; TC-01 ranks the verbatim shell entry first.

Verification

Check main this branch
lib tests 359 / 6 ignored 359 / 6 ignored
test_lib 35 36
test_main 2 / 1 ignored 4 / 0 ignored
make test-scenarios 9/9, needs network 9/9, offline
  • Every rewritten or new test was mutation-checked: the covered code path was broken on purpose (30+ mutations: filters, FTS, limit, ids, tags, access count, create-db, show) and each test failed; source restored and verified by hash.
  • JSON datasets verified row-by-row against databases seeded the old way (identical; hsearch differs only by the tag rename).
  • shellcheck, rustfmt and clippy clean on touched files.

Out of scope (possible follow-ups)

  • bkmr hsearch --json prints nothing instead of [] on an empty result.
  • bkmr add keeps unknown _x_ tags while load-json drops them.

@sysid
test(action): assert on the added bookmark, not seed row 1
ef9db97 
The shell-bookmark tests added a bookmark but read back id 1, which is
the Google row seeded by the first migration. They never exercised the
shell bookmark and still passed.

Read the bookmark by the id the repository assigned, and assert it is
the one added. Use a TempDir-backed file DB instead of :memory:, where
each pooled connection could open a separate database.
@sysid
test: declare test data explicitly instead of relying on seed rows
eff6241 
The first migration inserted 14 sample rows that every test DB
inherited implicitly. Remove the INSERT so tests start empty, and let
each test declare the rows it needs.

18 tests failed without the rows and now add their own data. 12 more
passed vacuously on an empty DB (assertions only inside loops over
results, or <=/>= comparisons); they now assert exact, non-empty
result sets. Each rewritten test was verified to fail when the code
path it covers is broken.

Drop the empty_bookmark_table() calls in tests that directly followed
setup_test_db(); a fresh DB is empty now.

Existing databases are unaffected: Diesel does not re-run applied
migrations, and create-db already emptied the table.
@sysid
refactor(cli): remove unreachable create_db duplicate
33ed77c 
main.rs routes the create-db command to handle_create_db_command
before any CLI dispatch, and cli::execute_command returns an error for
CreateDb. The copy in bookmark_commands.rs had no callers.
@sysid
fix(db): stop deleting migration seed rows on create-db
b65c642 
create-db emptied the bookmarks table after migrating, only to remove
the sample rows the first migration used to insert. Those rows are
gone, so drop the call and the now unused empty_bookmark_table().

Replace the ignored create-db stub in test_main with two guard tests:
create-db yields an empty database, and create-db --pre-fill yields
exactly the demo entries.
@sysid
test: define lsp and hsearch datasets as JSON shared by tests and scr…
51dd441 
…ipts

seed.sh built both datasets with dozens of `bkmr add` calls, and the
hsearch rows took their descriptions from live web pages, so the data
depended on the network and drifted over time.

Move both datasets to bkmr/tests/resources/datasets/*.json and load
them with `bkmr load-json`. Each entry keeps a group/why field that
documents its purpose; the importer ignores unknown fields. The
hsearch descriptions are the ones previously fetched, frozen.

load-json drops unknown system-style tags, so the hsearch tag
`_procedure_` becomes the plain tag `procedure`. verify.sh now expects
exactly 4 matches for the tag filter and exactly 2 for the limit
check, so an empty result can no longer pass.

A new stdio test loads the lsp dataset and asserts the completion
table documented in TESTING.md.
@sysid
docs(test): document explicit test data rule and datasets
56e56d2 
Add a "Test data" section to TESTING.md: every test starts from an
empty database, declares its rows, uses returned ids, and asserts
exact result sets. Describe the JSON datasets shared by seed.sh and
tests/lsp, and note that the LSP completion table is now asserted by
make test.

Follow the hsearch tag rename (_procedure_ -> procedure) and correct
two statements that were already wrong: group E entries are embedded,
and in TC-01 the verbatim shell entry ranks first.

CONTRIBUTING.md links the rule from its testing checklist.
@sysid
test(cli): assert displayed entries in show test
1dc3cdc 
The show test ran `bkmr show 1,2` against whatever the shared test DB
contained and only checked the exit code. show exits 0 even for
missing ids, so the test could not fail.

Seed a temp DB with three bookmarks, show two of them, and assert that
exactly those appear in the JSON and detailed output.
@sysid
sysid force-pushed the test/explicit-test-data branch from 85c2294 to 1dc3cdc Compare September 30, 2026 14:46
@sysid
test: make test-all fail reliably on any failing check
42f8c4b 
Under GNU make >= 3.82 .ONESHELL runs a recipe in one shell, so without
set -e only the last line's status counted in the test target. The
search regression check grepped for "bookmarks", which also matched
"Found 0 bookmarks"; assert the piped id list instead.
@sysid
sysid merged commit 1182886 into main Sep 30, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sysid