Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Earthquake Upgrade #255

Merged
merged 9 commits into from
Feb 2, 2025
Merged

Earthquake Upgrade #255

merged 9 commits into from
Feb 2, 2025

Conversation

kraftp
Copy link
Member

@kraftp kraftp commented Jan 30, 2025

Add unit tests to the Earthquake Tracker app to demonstrate how to unit-test a DBOS app.

@chuck-dbos
Copy link
Collaborator

This is a good start.

The one question I have is about 'conftest.py'... it looks like something that is complex enough that it could be subject to changing best practices over time, if not outright having issues. Is there a place where we can keep the "best one", so people adding testing to existing projects can grab it?

Documentation of the process for that (packages to install, template code and where to get it) is destined for a home in our docs at some point?

1 similar comment
@chuck-dbos
Copy link
Collaborator

This is a good start.

The one question I have is about 'conftest.py'... it looks like something that is complex enough that it could be subject to changing best practices over time, if not outright having issues. Is there a place where we can keep the "best one", so people adding testing to existing projects can grab it?

Documentation of the process for that (packages to install, template code and where to get it) is destined for a home in our docs at some point?

@kraftp
Copy link
Member Author

kraftp commented Jan 30, 2025

This is a good start.

The one question I have is about 'conftest.py'... it looks like something that is complex enough that it could be subject to changing best practices over time, if not outright having issues. Is there a place where we can keep the "best one", so people adding testing to existing projects can grab it?

Documentation of the process for that (packages to install, template code and where to get it) is destined for a home in our docs at some point?

I'm going to write a docs page for testing today. The conftest.py is complicated because I wanted to see if you could run Alembic migrations entirely programmatically to reset your database between tests. Turns out you can, but it's not pretty. Unfortunately that has nothing to do with DBOS--it's all on Alembic--so not sure how much we can improve this.

@chuck-dbos
Copy link
Collaborator

I'm going to write a docs page for testing today. The conftest.py is complicated because I wanted to see if you could run Alembic migrations entirely programmatically to reset your database between tests. Turns out you can, but it's not pretty. Unfortunately that has nothing to do with DBOS--it's all on Alembic--so not sure how much we can improve this.

It's not strictly about improving it. It's about leaving ourselves room to improve it and having it be simple for people to get the latest version.

@kraftp
Copy link
Member Author

kraftp commented Jan 30, 2025

I'm going to write a docs page for testing today. The conftest.py is complicated because I wanted to see if you could run Alembic migrations entirely programmatically to reset your database between tests. Turns out you can, but it's not pretty. Unfortunately that has nothing to do with DBOS--it's all on Alembic--so not sure how much we can improve this.

It's not strictly about improving it. It's about leaving ourselves room to improve it and having it be simple for people to get the latest version.

Got it! The obvious place to write down best practices like this is in the docs. I'll do that later today. Where else could we put it?

Copy link
Member

@qianl15 qianl15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good as a first step. We still have room to improve the DB connection setup, but maybe in a separate PR

@kraftp kraftp merged commit 96e3e86 into main Feb 2, 2025
2 checks passed
@kraftp kraftp deleted the kraftp/earthquake-upgrade branch February 2, 2025 23:59
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.

3 participants