-
Notifications
You must be signed in to change notification settings - Fork 273
Replace test.pl by Python-based implementation, possibly within pytest #1924
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
Comments
I've looked into pytest-xdist, which is a plugin for pytest to enable parallel running of tests. It seems to work. We have quite a small set of regression tests so it isn't a great comparison. Also, one of the tests takes much longer than the others (10:00 out of 11:39). Parallelising (with automatically chosen number of cores) reduces the time from 11:39 to 10:27. If I take out that long test then it reduces the time from 1:20 to 0:30. (All times reported are the "real" time obtained from |
@owen-jones-diffblue Thanks a lot for following up! Which test is it that vastly dominates the run-time? Maybe we've been working hard on solving the wrong problem... |
@tautschnig This was on a private repo, where the tests have already been written using pytest. And yes, we have plans for making that test a lot faster. |
Soon come ;-)
|
@karkhaz Have you rewritten test.pl using python? Or are you using a framework, like pytest? (I had to briefly check that it wasn't the 1st of April.) |
@owen-jones-diffblue It's not finished, but I'm rewriting it from scratch in python, and trying to ensure that it doesn't use any packages from outside the standard library. I actually thought about waiting until April to post the PR, just to be contrary; doing it twice just might make it a tradition... |
One thing to consider (if this is possible to fix in the new test runner) is that some of the current regression tests are not thread safe (e.g. if you try to distribute running the tests over multiple cpu's), particularly where one regression directory contains multiple '.desc' files. One such example is |
@karkhaz On April 1st you can announce that rewriting test.pl was too hard, so you're rewriting cbmc in perl instead 😄. |
@chrisr-diffblue are multiple .desc files the only situation where this happens? Would it suffice for me to parallelise all directories, but to always serialize .desc files within one directory? |
I think thats the main threading issue I'm aware of, so yes, that would probably be one approach - though that does still mean that if you want the test runner to leave behind the intermediate files (e.g. the |
Oh, and "Default: $TESTPL_JOBS if set" - are we keeping the "PL" for sentimental reasons, or should that be "TESTPY_JOBS" now :-) |
Thank you, a fresh directory for each .desc file sounds sensible---I'll do that. My job is a lot easier due to the plethora of regression tests, which serve to test regressions between test.pl and test.py as well as regressions of cbmc :) $TESTPL_JOBS: this was intentional. My rationale for this and other features is that I'd prefer not to break people's scripts. So at least for this first implementation, I'm going to strive to keep the command-line interface and other external settings identical. On the other hand, if there are long-standing annoyances with test.pl, this could certainly be an opportunity to finally fix them cleanly, so please feel free to point me to any issues that I might be able to resolve. The parallel .desc file issue is a nice example. |
As far as TESTPL_JOBS is concerned, I was mostly just being tongue-in-cheek - keeping as its current name seems entirely sensible. Its great to see someone finally tackling this and helping out everyone in the processes! |
Just a stray comment: One mild annoyance with the way test.pl right now is the colouring, which is unconditionally applied, which is fine if you're viewing the test log on travis or on your terminal, but a nuisance if you're writing to a file or viewing the log on AWS CodeBuild. I'd like to change the behaviour to
|
Closing since I guess this is no longer happening. Please reopen if you believe this is erroneous, or have the mythical python implementation working. |
See #1907 for discussion.
The text was updated successfully, but these errors were encountered: