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

Do not rely on sqlx database connection in client integration tests #53

Open
pohlm01 opened this issue Oct 29, 2024 · 3 comments
Open
Assignees
Labels
help wanted Extra attention is needed testing Relates to testing code

Comments

@pohlm01
Copy link
Member

pohlm01 commented Oct 29, 2024

The integration tests for event and program in the VEN library rely on a DB created by the sqlx testing framework. This is not ideal, especially if we want to test the integration of our VEN library with a 3rd party VTN. Instead, the tests should rely on a running VTN instance and connect to a predefined URL.
This also requires a predefined set of credentials with associated roles in the VTN, which the tests can rely on. Currently, they are created from the test_user_credentials.sql and applied in the CI before test execution.
The tests must also run serially instead of in parallel, to avoid interference between them.

The tests for resources and ven do that already, though there might be room for improvement on how to design the tests exactly. Especially, a failing test may currently leave the VTN/DB in a different state than before.

@pohlm01 pohlm01 converted this from a draft issue Oct 29, 2024
@pohlm01 pohlm01 added the testing Relates to testing code label Oct 29, 2024
@pohlm01 pohlm01 added the help wanted Extra attention is needed label Nov 11, 2024
@pohlm01
Copy link
Member Author

pohlm01 commented Nov 13, 2024

@joshuathayer thanks for your first contribution in #69! Feel free to pick this up as you suggested yourself. I'm happy to help you with any questions that might arise.
If you leave a (short) comment in this thread, I can assign you to the issue to make it obvious to others that you are working on it.

@joshuathayer
Copy link
Contributor

Sounds good, I'll be happy to take a look at this in the coming days.

In the issue description, you mention "the tests for resources and ven do that already"- do you mean, they're set up to call a running instance of a VTN? If that's the case, I suspect I can take a look at those tests to see how event and program can be altered not to require direct DB access. Maybe as a followup we'll try to make sure failing tests don't leave the VTN in an inconsistent state.

@pohlm01
Copy link
Member Author

pohlm01 commented Nov 14, 2024

Yes, you understand the description correctly. The reason is that we added the implementation and tests for resource and ven later than for program and event (reports are missing tests altogether, see #75). At that later movement, when we added resource and ven tests, we made sure not to depend on the DB connection in the client library. So you can definitely draw - possibly a lot of - inspiration from those implementations if you want.
We appreciate any improvement over the currently DB-dependent tests. Even if they don't feel perfect yet, we can always improve further later. I recommend opening a (draft) PR already in an early state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed testing Relates to testing code
Projects
Status: In Progress
Development

No branches or pull requests

2 participants