feat: add BDD test fixtures for API interactions and database setup

[jirhiker]

Why

This PR addresses the following problem / context:

• Use bullet points here

How

Implementation summary - the following was changed / added / removed:

• Use bullet points here

Notes

Any special considerations, workarounds, or follow-up work to note?

• Use bullet points here

[codecov-commenter]

Codecov Report

❌ Patch coverage is 36.36364% with 7 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
services/observation_helper.py	0.00%	4 Missing ⚠️
transfers/transfer.py	0.00%	3 Missing ⚠️

Files with missing lines	Coverage Δ
core/app.py	29.62% <ø> (-0.50%)	⬇️
core/initializers.py	86.31% <100.00%> (+0.29%)	⬆️
main.py	84.61% <100.00%> (+2.79%)	⬆️
transfers/transfer.py	40.81% <0.00%> (-7.34%)	⬇️
services/observation_helper.py	91.01% <0.00%> (-1.04%)	⬇️

... and 4 files with indirect coverage changes

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[jacob-a-brown]

It makes sense to me to tear down all of the objects after the tests have run. If not, they'll persist and cause issues down the line, as well as hide potential API behavior.

I think it'd be good to uncomment the after_all, or remove the if statement in before_all to ensure that the tests always start with a blank database.

[jacob-a-brown]

One of the things that I appreciated about pydantic's fixtures is the ease of calling and identifying them. What're your thoughts on storing them in a dictionary organized by object name and then ID?

e.g.

context.objects = {
    "thing": {
        1: {water well with ID 1}
        2: [water well with ID 2}
    },
    "location": {
        1: {location with ID 1}
       ...
    }
    ...
}

Then when I want to retrieve the thing with ID 1 I could call context.objects["thing"][1] and have access to it (and its attributes). Single table inheritance may make this slightly more complicated as we add other data types, but the complications wouldn't be all that great

[jirhiker]

I agree that any data added to the db during a step function should be removed, however I don't see the point in removing the seed data. Deleting the seed data and reloading actually has the unintended consequence of changing the id so that objects can no longer simply be referred to by id. e.g. when sensor.id=1 is removed from the db and reloaded it is assigned sensor.id=2

The add_ functions are all idempotent which will address " If not, they'll persist and cause issues down the line, as well as hide potential API behavior."

[jirhiker]

    "thing": {
        1: {water well with ID 1}
        2: [water well with ID 2}
    },
    "location": {
        1: {location with ID 1}
       ...
    }
    ...
}```
this is fine

[jacob-a-brown]

If run_bdd.sh is run more than once, the same records will be added to the database n time. This may not cause issues for all models, but there are some that have unique constraints, like sensors, which will cause database errors.

I don't think that we need to know about specific IDs (1, 2, 3, ...), we just need to know that an object is in the database and that we can retrieve and use its ID (like how the pytest fixtures are currently used). Since the records are persisted in a list it won't matter what their IDs are. The test can invoke context.objects["wells"][0].id to get the relevant ID and we won't need to worry about/know its value.

[jirhiker]

"If run_bdd.sh is run more than once, the same records will be added to the database n time[s]" That is not true. all the add_ functions test if the object is in the database before adding.

[jacob-a-brown]

I missed that 🤦, then this should work well 👍

[jacob-a-brown]

Should responses be stored in a dictionary called context.responses, too? That way multiple requests can be made per scenario (such as to POST /thing/water-well and GET /thing/water-well/{thing_id}). That way we aren't restricted to one request per scenario. If this is an agreeable way to proceed, these can be changed to

for response in context.responses.values():
    assert response.status_code == 200

or something like that

[jirhiker]

Do we have an example of the need for multiple requests for a given scenario? Perhaps we should consider writing our scenarios such that only one request is necessary