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

Remove non-lmdb stores from unit tests #1566

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

G-D-Petrov
Copy link
Collaborator

Reference Issues/PRs

Fixes #1564

What does this implement or fix?

It remove all simulators from the unit tests (e.g. S3 moto, azurite, mongo)

TODO - make sure that we don't need these tests, if we don - add them to the integration tests

@G-D-Petrov G-D-Petrov linked an issue May 10, 2024 that may be closed by this pull request
@G-D-Petrov
Copy link
Collaborator Author

G-D-Petrov commented May 10, 2024

Moved the test_fork test to integration as it seems sensible to test this with a s3 simulation at least
With these changes, locally the unit tests run in ~2min in comperisson to ~7min before them with ARCTICDB_FAST_TEST_ONLY=1
@alexowens90 - do we need to test the QB aggregations with s3/azure simulations or is lmdb enough?

@willdealtry
Copy link
Collaborator

Are we sure this is a good idea? Azurite and Mongo are quite irritating in that they mostly fail locally, but S3 is the storage that the vast majority of our users actually use. This means we're removing more than half the tests in the whole project from the one storage that actually matters to the majority of people, which seems like a high price to pay for knocking a few minutes off a CI run

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.

Make sure that we can run unit tests locally
3 participants