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

basic tests #584

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from
Open

basic tests #584

wants to merge 5 commits into from

Conversation

austinmilt
Copy link
Contributor

This PR adds initial setup for running tests against DB queries. Right now it's primarily testing the collections queries. This will ensure that if fields required for deserialing Nft change, the build will fail.

mpwsh
mpwsh previously approved these changes Jul 18, 2022
Copy link
Contributor

@ray-kast ray-kast left a comment

Choose a reason for hiding this comment

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

Gonna make some ergonomic fixes and review our options for scaffolding tests

@ray-kast ray-kast force-pushed the austin/sql-query-test branch 4 times, most recently from 3951ed4 to 045878d Compare August 5, 2022 01:16
@@ -0,0 +1,27 @@
delete from metadatas where address = any(array [
Copy link
Contributor

Choose a reason for hiding this comment

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

Normally unit tests involving databases are done in transaction that leave the db empty after each unit test but since we are only doing read operations seeding with a shared set of data works for our use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seconding this. I like the migration setup. It would be awesome to be able to define a migration per test in such a way that we could run up and down for a specific migration for each test, just to keep the db minimally populated with test-specific data.

This is a great start, though!

Copy link
Contributor Author

@austinmilt austinmilt left a comment

Choose a reason for hiding this comment

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

looks awesome!

Comment on lines +230 to +231
dotenv::from_filename(".env.dev").expect("Failed to load .env.dev");
dotenv::from_filename(".env").expect("Failed to load .env");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should this also read from .env.local?

Copy link
Contributor

@ray-kast ray-kast Aug 5, 2022

Choose a reason for hiding this comment

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

I chose not to as an attempt to enforce consistency in the test setup. Unsure if that will pay off, but if we need .env.local at some point I'll cave and add it.

@@ -0,0 +1,27 @@
delete from metadatas where address = any(array [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seconding this. I like the migration setup. It would be awesome to be able to define a migration per test in such a way that we could run up and down for a specific migration for each test, just to keep the db minimally populated with test-specific data.

This is a great start, though!

@ray-kast ray-kast force-pushed the austin/sql-query-test branch 3 times, most recently from 53f46fa to 080e74f Compare August 12, 2022 19:35
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.

4 participants