Skip to content

No catalyst for db#1641

Merged
Ericson2314 merged 2 commits intoNixOS:masterfrom
obsidiansystems:no-catalyst-for-db
Apr 4, 2026
Merged

No catalyst for db#1641
Ericson2314 merged 2 commits intoNixOS:masterfrom
obsidiansystems:no-catalyst-for-db

Conversation

@Ericson2314
Copy link
Copy Markdown
Member

See each commit for details. We're making the tests operate in both a more consistent and less indirect manner.

…ydra_setup`

Ever since `HydraTestContext` was introduced (eca09bc, 2021) to fix
teardown errors, its `db()` had created the root user on first access —
making the older `hydra_setup($db)` idiom redundant. But not every test
was converted from the `hydra_setup` way of doing things, and also
subsequent test files continued copying the older pattern of
`Hydra::Model::DB->new` + `hydra_setup($db)` from neighboring tests.
Also `createBaseJobset` internally constructed its own
`Hydra::Model::DB->new` handle.

Now, Migrate all tests to the `$ctx->db()` / `$ctx{context}->db()`
pattern and pass the `$db` handle explicitly to `createBaseJobset` and
`createJobsetWithOneInput` instead of letting them create their own.
…::Model::DB`

In general, it is better not to use the app to test the app. Relying on
just the database framework but not also the web framework is a strict
improvement.

`Hydra::Model::DB` is a `Catalyst::Model::DBIC::Schema` subclass — a
Catalyst adapter around DBIC. Tests never needed Catalyst; they just
needed a database handle. The original tests (890a786, 2011) connected
directly via `openHydraDB`. The Catalyst dependency was introduced in
fa62c8b (2012) when that was replaced with `Hydra::Model::DB->new`,
and it stuck.

Now that all test files obtain their `$db` from `HydraTestContext::db()`,
we can switch that single callsite from `Hydra::Model::DB->new()` to
`Hydra::Schema->connect(...)`. The `DESTROY` method is adjusted
accordingly: `->storage->disconnect()` instead of
`->schema->storage->disconnect()`, since we now hold the schema object
directly rather than a Catalyst model wrapper.
@Ericson2314 Ericson2314 enabled auto-merge April 4, 2026 21:31
@Ericson2314 Ericson2314 added this pull request to the merge queue Apr 4, 2026
Merged via the queue into NixOS:master with commit 461fefe Apr 4, 2026
2 checks passed
@Ericson2314 Ericson2314 deleted the no-catalyst-for-db branch April 4, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant