[Lldb-commits] [PATCH] D70314: [lldb] Add better test commands for expression evaluation

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D70314#1747850 <https://reviews.llvm.org/D70314#1747850>, @teemperor wrote:

> In D70314#1747711 <https://reviews.llvm.org/D70314#1747711>, @labath wrote:
>
> > Semi-random idea: Instead of running the expression through the command line and then unpacking the result, should we just use the appropriate SBFrame api here (EvaluateExpression/GetValueForVariablePath)?
>
>
> That's one of the APIs we need to test, but we need to test all of them to prevent regressions like D67803 <https://reviews.llvm.org/D67803>. At least `frame var` and `expr` behave very differently (EvaluateExpression should be similar to `expr` but I never looked at that code path). So the idea is to have one command that tests all these APIs and if we can't do that (for example because the expression can't be repeated with the same result), at least make that safer/easier to do.


I agree we need to test all API's, but the testing strategies can differ. I am aware that "frame variable" and "expression" are very different. However, SBFrame::EvaluateExpression is not that different from the "expression" command -- in fact, I'd say that it's indistinguishable as for the stuff you're interested in testing here. And same goes for "frame variable" and SBFrame::GetValueForVariablePath.

So, I think we could just use one of the two mechanisms for the general evaluation test, and then have different tests for which test the unique properties of the specific mechanisms (e.g., command line option parsing). I certainly don't see a reason to run the same expression both through the command line and SB apis "just in case".

I'm thinking/hoping that the SB api approach would be a better fit for the "run this expression and see what you get" kinds of tests, because it avoids the need to reverse-engineer the values/types out of the textual output.



================
Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:2480-2482
+        self.expect_expr(var, result_value=result_value, result_type=result_type, run_type=self.ExpressionCommandType.EXPRESSION)
+        self.expect_expr(var, result_value=result_value, result_type=result_type, run_type=self.ExpressionCommandType.FRAME_VAR)
+        self.expect_expr(var, result_value=result_value, result_type=result_type, run_type=self.ExpressionCommandType.EXPRESSION)
----------------
I'm also not sold on the idea of running the same expression multiple times just because we've had some bug that would've been caught by that in the past. Lldb already does a lot more combinatorial testing than anything else in llvm. I don't think that adding more of it is the solution to any stability problem.

If there's some tricky aspect of combining "frame variable" and "expression" commands then we should have a separate test for that. I'd be much happier to have one or two tests that run a single expression a hundred times than putting the repetition in every test and hoping the shotgun effect of that will catch something.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70314/new/

https://reviews.llvm.org/D70314