Re: [PATCH v4 7/8] tracing/selftests: Add tests for hist trigger expression parsing

[Masami Hiramatsu]

On Tue, 26 Oct 2021 07:28:39 -0700
Kalesh Singh <kaleshsingh@xxxxxxxxxx> wrote:

> On Tue, Oct 26, 2021 at 5:43 AM Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
> >
> > Hi Kalesh,
> >
> > On Mon, 25 Oct 2021 13:08:39 -0700
> > Kalesh Singh <kaleshsingh@xxxxxxxxxx> wrote:
> >
> > > Add tests for the parsing of hist trigger expressions; and to
> > > validate expression evaluation.
> > >
> > > Signed-off-by: Kalesh Singh <kaleshsingh@xxxxxxxxxx>
> > > Reviewed-by: Namhyung Kim <namhyung@xxxxxxxxxx>
> > > ---
> > >
> > > Changes in v3:
> > >   - Remove .sym-offset error check tests
> > >
> > > Changes in v2:
> > >   - Add Namhyung's Reviewed-by
> > >   - Update comment to clarify err_pos in "Too many subexpressions" test
> > >
> > >
> > >  .../testing/selftests/ftrace/test.d/functions |  4 +-
> > >  .../trigger/trigger-hist-expressions.tc       | 72 +++++++++++++++++++
> > >  2 files changed, 74 insertions(+), 2 deletions(-)
> > >  create mode 100644 tools/testing/selftests/ftrace/test.d/trigger/trigger-hist-expressions.tc
> > >
> > > diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions
> > > index 000fd05e84b1..1855a63559ad 100644
> > > --- a/tools/testing/selftests/ftrace/test.d/functions
> > > +++ b/tools/testing/selftests/ftrace/test.d/functions
> > > @@ -16,13 +16,13 @@ reset_tracer() { # reset the current tracer
> > >
> > >  reset_trigger_file() {
> > >      # remove action triggers first
> > > -    grep -H ':on[^:]*(' $@ |
> > > +    grep -H ':on[^:]*(' $@ | tac |
> > >      while read line; do
> > >          cmd=`echo $line | cut -f2- -d: | cut -f1 -d"["`
> > >       file=`echo $line | cut -f1 -d:`
> > >       echo "!$cmd" >> $file
> > >      done
> > > -    grep -Hv ^# $@ |
> > > +    grep -Hv ^# $@ | tac |
> > >      while read line; do
> > >          cmd=`echo $line | cut -f2- -d: | cut -f1 -d"["`
> > >       file=`echo $line | cut -f1 -d:`
> >
> > If this update has any meaning, please make a separate patch for this part.
> 
> Hi Masami,
> 
> Thanks for the feedback. The above change is to ensure we remove
> triggers in the reverse order that we created them - important when
> one trigger depends on another.

Hi Kalesh,
That's a good reason to make this an independent patch :)

> I can split it out into a separate
> patch and will add a README pattern check to the requires tag for
> these tests.

Thank you!



-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>