-
Notifications
You must be signed in to change notification settings - Fork 72
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
Use the Test Name for the dbname when running unit tests #353
Conversation
Add a method that allows the test name to be part of the DB name. This makes it easier to associate a given test run with a given test. Otherwise, some of the tests will overwrite the old data from a previous test.
closes #273 |
atm, each suite running in the same process creates one dir and each test in it is using the same dir. these changes create dir for each test inside the suite which will create much more leftovers as can be seen in compaction_service_test. ( DestroyDB doesnt clean up the dir created by the remote compaction. ) |
I agree the tests should do a better job of cleaning up after themselves. This PR is not about that at all however. It is meant to allow one to determine what test left what around. It is also ensures that the parallel tests use separate directories and can eliminate some of the other information (pid/tid, etc) that are built into the directory/DB names now. The goal would be to simplify the naming in a future PR. |
what i meant is this PR causes a huge increase in the leftovers (undeleted files and dirs in the test dir (rocksdbtest-1000)) which seems like a problem so it might be addressed here or in a subsequent PR but the point is to address it. |
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.
LGTM, just needs rewording of the commit msg and add the PR number
fuzzer error doesnt seem to be related to the changes. |
@mrambacher , i've checked the size of the leftovers and heres what i found: (using the makefile) parallel make check: serial make check (J=1) do you have an idea why theres a difference between the size of the leftovers of the serial run vs the parallel? could the parallel version not run all the tests that the serial does? |
just noticed that @isaac-io was working on it in isaac/test-cleanup |
we've agreed to add the test executable name before so that its easy to identify the suite to which an individual test belongs to. we've also agreed to address the cleanup only if the need arises. |
theres also a problem with the fuzzer caused by a running the CI on a branch that exists in a different repo. @maxb-io is on it. |
Adding the file name is more complex than I was hoping and I would like to address that in a subsequent PR where we potentially can also clean up the test name further. |
sure np |
just needs to be a single commit with a proper msg |
@mrambacher , can you fix this?
|
on hold until commit msg is fixed |
Add a method that allows the test name to be part of the DB name. This makes it easier to associate a given test run with a given test. Otherwise, some of the tests will overwrite the old data from a previous test.
Add a method that allows the test name to be part of the DB name. This makes it easier to associate a given test run with a given test. Otherwise, some of the tests will overwrite the old data from a previous test.
Add a method that allows the test name to be part of the DB name. This makes it easier to associate a given test run with a given test. Otherwise, some of the tests will overwrite the old data from a previous test.
Add a method that allows the test name to be part of the DB name. This makes it easier to associate a given test run with a given test. Otherwise, some of the tests will overwrite the old data from a previous test.
Add a method that allows the test name to be part of the DB name. This makes it easier to associate a given test run with a given test. Otherwise, some of the tests will overwrite the old data from a previous test.
Add a method that allows the test name to be part of the DB name. This makes it easier to associate a given test run with a given test. Otherwise, some of the tests will overwrite the old data from a previous test.
This PR is based on the following RocksDB PR.
The name of the database directory can be based on the name of the test itself. This allows multiple tests in the same test class to use different database directories rather than all going to the same or similar directories.
This change can also help in debugging and KEEP_DB tests, as the directory after the test run can be spotted by its name (which is based on the name of the test).