Fix issue with Database class#276
Conversation
|
@ckendrick can you please provide an example test that demonstrates the issue? |
|
@dylan-copeland this issue should occur if you run any example from the current upstream branch during the offline phase. This should happen to any code that calls |
| Database::create(const std::string& file_name) | ||
| { | ||
| std::cout << "Creating file: " << file_name << std::endl; | ||
| return true; |
There was a problem hiding this comment.
I think this change is already made at #274 . I'm open to @dylan-copeland 's suggestion.
|
@dylan-copeland would you review this PR again? |
|
@dreamer2368 do you approve this PR? |
|
@dylan-copeland we need your review again on this PR. |
| // Finalize MPI and return. | ||
| MPI_Finalize(); | ||
| return !status; | ||
| return 0; |
There was a problem hiding this comment.
It looks like the goal of this test is just to run without failing? There is nothing checked, like in test_Matrix.cpp where EXPECT* functions are called. Is this the intention?
There was a problem hiding this comment.
Yes this test was added to the CI only intending to be a quick check if an error occurred with endSamples()/writeSnapshot(). The issue that caused this problem has since been fixed, but the only test I could find that uses these functions is in test_HDFDatabase.cpp which is restricted to parallel HDF5 builds only.
In the future, we should provide a serial test in test_HDFDatabase.cpp and remove this test (and other outdated/unused tests).
4 participants
This adds a return statement to the base Database functions
open()andcreate(). Without these, calls toendSamples()andwriteSnapshot()would hang indefinitely printing newlines to screen.This also adds a test to the CI that calls
writeSnapshot()to catch this behavior in the future.