-
Notifications
You must be signed in to change notification settings - Fork 5
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
Run tests against PostgreSQL, fix compatibility #170
Changes from 1 commit
6b42328
5a6ccd4
63b0df2
2a0c0df
51908c5
21e41b6
b5276a8
25567a1
bad9952
3dada54
d21efa5
e22f704
a598bfe
a0f86e1
8c5199c
0935387
1aad8f0
413cfbb
590dbd3
1ca7976
05d2ad0
620f34b
4236230
f34b4e7
3d4acc2
971cf99
7894fd9
275e19b
ae6a6c4
c2b657f
e77ee40
4459eff
50e7168
2abfd81
06ac905
79637c9
db0cccc
89f0e2c
ba3a7ff
38bf7f8
1884016
d97e8b9
3980942
7ab5a45
f1cb76d
c3ba146
eb762f2
12a1028
2cde8b9
213709b
df1d9b9
89efbd7
ef08996
65130de
c51434f
d2129dd
7c1275c
818139d
6778ba4
0c0fbcc
e7fd225
5369d26
cbad270
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -248,7 +248,7 @@ impl TestDatabaseRunner { | |
pool.close().await; | ||
|
||
// Panic here when test failed to propagate it further | ||
assert!(result.is_ok()); | ||
result.unwrap(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this will make There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, DOH!!! Yeh, hopefully, why didn't I think of that... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've tested it, it works again 👍 |
||
}); | ||
} | ||
} | ||
|
@@ -303,9 +303,6 @@ pub struct TestDatabase { | |
/// | ||
/// Returns a `TestDatabase` containing storage provider instance, a vector of key pairs for all | ||
/// authors in the db, and a vector of the ids for all documents. | ||
/// | ||
/// Note: This helper should not be used directly in tests as it does not assure that the database | ||
/// connection will be closed after the test finished or failed. Please use `test_db` instead. | ||
async fn create_test_db( | ||
no_of_entries: usize, | ||
no_of_authors: usize, | ||
|
@@ -403,10 +400,6 @@ async fn create_test_db( | |
} | ||
|
||
/// Create test database. | ||
/// | ||
/// Note: Make sure to close the connection to the database when using this method in tests. Please | ||
/// have a look at the `TestDatabaseRunner` in case you need a tool which does that automatically | ||
/// for you. | ||
async fn initialize_db() -> Pool { | ||
// Reset database first | ||
drop_database().await; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I understand this docstring. Propagate the error further? Wouldn't this get triggered exactly when an error is propagated further and not when it is not? If this enables
#[should_panic]
that could also be nice to mention here for whoever edits this code next.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if I understand your question, but I'll try to explain it from another angle: Panicks in tasks are not propagated further ("unwinded"), they just crash the task. To allow the test runner to understand that something happend we have to take the task handle, unwrap it to inform the "parent" about its failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function of this is to make the test fail when it should :-D