-
Notifications
You must be signed in to change notification settings - Fork 232
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
databaseTest could collect all errors instead of reporting just the first #1549
Comments
I've made a branch called databasetestimprove where I have a commit that I believe should enable this. So far I've confirmed that it passes tests on master. I haven't tested beyond that yet, but feel free to try it out. |
That seems to do the trick (mostly).
|
Also, we log too many debug messages by default, which makes it hard to see the important stuff. Can we consider reducing (or commenting out) database-related debug messages? |
I feel like at some time I definitely agreed with Alon, but now I can't remember what made this happen. Looking at this databasetest.log (made with
followed by the captured error messages which are needed to debug. Not too much distraction. Is it a problem on travis? or certain tests print more messages? or if you run with Anyway, I'm not going to argue against an audit of uses of print / logging.debug / logging.info / logging.error |
Ok, most of those seem to just be issues where failing one assert (that used to end the test) makes it so the second/third tests can't be done. I've updated the branch with fixes for the cases identified here. |
So there isn't any debug code in the database tests, but there is in some of the database functions it calls so when some of the database tests fail the debug from those functions show up. I saw some of that debugging this test. Perhaps we can add that as a smaller part of the coming hackathon. |
Ahh, that makes sense (where the debug messages come from). I knew I'd seen it some time. Could remove them, and/or change the logging level in databaseTest? |
#1597 added --nocapture and --nologcapture so we no longer get all of the debugging messages when tests fail. |
Motivation or Problem
It takes 8 minutes for me to run
make test-database
locally, and ~30 minutes on travis. When there are many mistakes to be fixed in the database, it's frustrating to have them listed one at a time and have to run the whole test suite again to reveal the next problem.Desired Solution
Some of the tests (checking units, checking rates are reasonable) already collect up a list of failures and report them all. This is great. It'd be nice if all the other tests (nodes can be placed in trees, nodes are not duplicates, etc. etc.) did the same thing: log all the problems (with
logging.error
) and then fail the test at the end, rather than failing the test at the first problem.The text was updated successfully, but these errors were encountered: