- add decorator functions to skip test on specific os, architecture and version of comipler
- skip failing test with gcc-4.9.2 on linux
Details
Diff Detail
Event Timeline
test/lldbtest.py | ||
---|---|---|
740 | Could we just name this function "skipIf" and use it directly to decorate tests? May be we could eliminate all skipIf* decorators in favor of this, but I have not thought through that thoroughly. Atleast, it seems to me that things like skipIfLinuxGcc, skipIfGcc and skipIfClang can be eliminated. You will ofcourse have to change the logic in the 'fn' function below. And ofcourse you need not have to remove/modify the existing decorators in this patch itself. One thing we might loose is the ability to "git grep xxx" to get all tests decorated with xxx. |
test/lldbtest.py | ||
---|---|---|
740 | yes, this function could be used to decorate tests directly as is. My understanding is that by providing a various of wrapper functions like skipIfLinuxGcc, skipIfClang, we got better readability of code, and also as you mentioned, it's easier to find all tests decorated with certain compiler or host. We could definitely re-factor current skipif* functions to call this function instead of having common code in each function. |
test/lldbtest.py | ||
---|---|---|
740 | Please note that I am not taking ownership of this review. I do not have strong opinions about the changes and would like others to chime in and drive. IMO, @skipIfLinuxGcc('bugnumber', ['>=', '4.9'], ['i386']) is not any more readable than @skipIf("bn", "linux", "gcc", ['>=','4.9'], ['i386','x86']). Also, one can still git grep, but have to put slightly more effort. For example, instead of doing git grep skipIfLinux one will now have to do git grep "skipIf(.*linux.*)" |
Slightly off topic, but I would like to ask what is the guideline for
using skipIf vs. expectedFailure? Do we have one? (If not maybe we
should make one)
I was under the impression that expectedFailureXXX is to be used for
tests that are failing due to a bug in LLDB, whereas skipIfXXX are for
situations where the test simply does not apply to the platform
environment in question (e.g. debugserver tests on linux, etc). If
that is the case, then these tests should be XFAILed rather then
skipped.
Is that so ?
For the readability issue, I would propose the following syntax:
@skipIf("bugnumber", lambda x: compilerIs("gcc>=4.9") &&
platformIs("linux") && architectureIs("aarch64", "x86_64"))
I think this would be more versatile that the previous solutions, and
you don't need to litter the code with so many decorators.
What do you think about that?
cheers,
pl
I think your understanding is correct, based on pytest documentation:
"A skip means that you expect your test to pass unless the environment (e.g. wrong Python interpreter, missing dependency) prevents it to run. And xfail means that your test can run but you expect it to fail because there is an implementation problem."
Do we know if all these failures are caused by lldb bugs or possibly gcc-4.9.2 bugs?
Another difference I would like to point out is that, the tests will still be run if marked as xfail.
And sometimes it takes longer to run failing tests. For example, TestMiExec.py takes 164s to complete with 4 errors(with gcc4.9.2 and i386). It makes the overall test time longer. That's why I chose skip over xfail here.
Using lambda functions is a good idea. We could potentially combine multiple decorators to one.
For example, @skipIf('bn', lambda:(platform(['FreeBSD'])) or (platform(['linux']) and compiler('gcc')))
Please let me know if anyone has different ideas.
We could definitely re-factor current skipif* functions to call this function instead of having common code in each function.
Yes, we should at least do that. When I added FreeBSD support to the tests I spent some time to refactor the expectedFailure cases to avoid the copypasta, but for some reason didn't take on the skipIf tests. I don't have a strong opinion on skipIfFreeBSD vs skipIf(... "FreeBSD" ...)
I was under the impression that expectedFailureXXX is to be used for tests that are failing due to a bug in LLDB, whereas skipIfXXX are for situations where the test simply does not apply to the platform environment in question (e.g. debugserver tests on linux, etc).
Yes, this is consistent with my understanding, with the caveat that we also (ab)use skip for tests that take excessively long to fail / timeout, or that consistently crash (as that case is not conveniently handled by the infrastructure).
Please make suggested changes and submit.
test/api/multithreaded/TestMultithreaded.py | ||
---|---|---|
36 | Agreeing with previous comments, these should be expectedFailureLinuxGcc Since Gcc tests aren't run on OSX, should we assume these are just expectedFailureGcc? | |
test/functionalities/inferior-crashing/TestInferiorCrashing.py | ||
82 | Did you forget brackets? archs=["i386"] |
Mark these tests with xfail instead of skip
-Add decorator to xfail specified platform/compiler/architecture
Since the code will check against target platform, and we don't intend to xfail those tests with Android target, I added new xfail function to take platform and compiler and architecture.
test/lldbtest.py | ||
---|---|---|
734 | If this is dead code currently, I would suggest you use it atleast once. TestVectorTypesFormatting is a good candidate that I know of. |
Add one usage of the new skipIf decorator as suggested.
Add more description of decorator usage.
Agreeing with previous comments, these should be expectedFailureLinuxGcc
Since Gcc tests aren't run on OSX, should we assume these are just expectedFailureGcc?