Persistent mock database #637
Conversation
AlexandraRoatis
requested review from
arajasek,
AionJayT,
aionick and
aion-kelvin
AlexandraRoatis
force-pushed
the
db-mock
branch
from
dbad90c to
59e597d
Compare
|
Since persistence is loosely defined for the class |
| File dbFile = new File(path); | ||
|
|
||
| if (dbFile.exists()) { | ||
| BufferedReader reader = null; |
There was a problem hiding this comment.
This little block could be cleaned up a bit using a try-with-resources statement
| } | ||
|
|
||
| private static final byte[] convertToByteArray(String commaSeparatedNumbers) { | ||
| String[] numbers = |
There was a problem hiding this comment.
The end index is exclusive in substring, not sure if this is intentional that you want to chop off the last character or not, just checking
There was a problem hiding this comment.
I'm eliminating the [ and ] that are added when calling Arrays.toString(...) which is used for storing the data in the file.
|
|
||
| FileWriter writer = null; | ||
|
|
||
| try { |
There was a problem hiding this comment.
I'd do a try-with-resources here
| @@ -575,8 +579,9 @@ public void testDrop() { | |||
| } | |||
|
|
|||
| @Test | |||
| /** This test is non-deterministic and may fail. If it does, re-run the test suite. */ | |||
There was a problem hiding this comment.
how do we know if failure is problematic or not?
There was a problem hiding this comment.
Personally, I don't see much value in it. It's a legacy test. I almost removed it.
There was a problem hiding this comment.
I got confirmation that the test can be ignored.
| if (reader != null) { | ||
| reader.close(); | ||
| } | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
print e.printStackTrace()?
Description
Database implementation to be used in testing when we want to check behavior for persistent databases. The class
PersistentMockDBinherits most of the functionality fromMockDBwith the addition that data is read from a file on disk at open (if the file exists) and it is stored to disk at close.Fixes Issue #636.
Type of change
Insert x into the following checkboxes to confirm (eg. [x]):
Testing
Please describe the tests you used to validate this pull request. Provide any relevant details for test configurations as well as any instructions to reproduce these results.
Verification
Insert x into the following checkboxes to confirm (eg. [x]):