On Fri, Apr 03, 2015 at 10:48:03AM +0300, Pekka Paalanen wrote:
> On Thu, 2 Apr 2015 19:35:52 -0700
> Bryce Harrington <[email protected]> wrote:
>
> > On Thu, Apr 02, 2015 at 02:13:36PM +0300, Pekka Paalanen wrote:
> > > On Wed, 1 Apr 2015 19:17:07 -0700
> > > Bryce Harrington <[email protected]> wrote:
> > >
> > > > There are only minor differences in the syntax for how weston
> > > > is invoked for .so/.la tests vs. other tests, but it's hard
> > > > to spot them. Refactor the command itself out, so it becomes
> > > > clearer what the difference is.
> > > >
> > > > Signed-off-by: Bryce Harrington <[email protected]>
> > > > ---
> > > > tests/weston-tests-env | 35
> > > > +++++++++++++++++------------------ 1 file changed, 17
> > > > insertions(+), 18 deletions(-)
> > > >
> > > > diff --git a/tests/weston-tests-env b/tests/weston-tests-env
> > > > index 3cb3073..173b6eb 100755
> > > > --- a/tests/weston-tests-env
> > > > +++ b/tests/weston-tests-env
> > > > @@ -37,24 +37,23 @@ fi
> > > >
> > > > case $TEST_FILE in
> > > > *.la|*.so)
> > > > - WESTON_BUILD_DIR=$abs_builddir \
> > > > - $WESTON --backend=$MODDIR/$BACKEND \
> > > > - $CONFIG \
> > > > - --shell=$SHELL_PLUGIN \
> > > > - --socket=test-${TEST_NAME} \
> > > > -
> > > > --modules=$MODDIR/${TEST_FILE/.la/.so},$XWAYLAND_PLUGIN
> > > > \
> > > > - --log="$SERVERLOG" \
> > > > - &> "$OUTLOG"
> > > > + TEST_MODULE=$MODDIR/${TEST_FILE/.la/.so}
> > > > + client=
> > > > ;;
> > > > *)
> > > > - WESTON_BUILD_DIR=$abs_builddir \
> > > > -
> > > > WESTON_TEST_CLIENT_PATH=$abs_builddir/$TEST_FILE $WESTON \
> > > > - --socket=test-${TEST_NAME} \
> > > > - --backend=$MODDIR/$BACKEND \
> > > > - $CONFIG \
> > > > - --shell=$SHELL_PLUGIN \
> > > > - --log="$SERVERLOG" \
> > > > - --modules=$TEST_PLUGIN,$XWAYLAND_PLUGIN
> > > > \
> > > > - $($abs_builddir/$TEST_FILE --params)
> > > > \
> > > > - &> "$OUTLOG"
> > > > + export
> > > > WESTON_TEST_CLIENT_PATH=$abs_builddir/$TEST_FILE
> > > > + TEST_MODULE=$TEST_PLUGIN
> > > > + client=$($WESTON_TEST_CLIENT_PATH --params)
> > > > esac
> > > > +
> > > > +export WESTON_BUILD_DIR=$abs_builddir
> > > > +$WESTON \
> > > > + --shell=$SHELL_PLUGIN \
> > > > + --socket=test-${TEST_NAME} \
> > > > + --modules=$TEST_MODULE,$XWAYLAND_PLUGIN \
> > > > + --backend=$MODDIR/$BACKEND \
> > > > + --log="$SERVERLOG" \
> > > > + $CONFIG \
> > > > + $client \
> > > > + &> "$OUTLOG"
> > > > +
> > >
> > > I suppose this is fine in principle, I just have to adapt this
> > > to it:
> > > http://cgit.collabora.com/git/user/pq/weston.git/tree/tests/weston-tests-env?h=ivi-test-5
> > >
> > > ivi-*.la needs to use --ivi-module instead of --modules to load
> > > the test plugin.
> > >
> > > ivi-*.la and ivi-*.weston need a special config file (all tests
> > > use the same file), must not load xwayland.so, and need
> > > ivi-shell.so as the shell.
> > >
> > > It's going to be messy in any case, I'm afraid. Any good ideas?
> > >
> > >
> > > Thanks,
> > > pq
> >
> > Following on from the patchset I just posted, actually the ivi
> > customizations integrate fairly cleanly.
> >
> > I didn't know if you want to also look in the src tree, but the
> > plumbing is all there now if you do.
>
> Hi,
>
> where is that tree?
${abs_top_srcdir}/tests
> > From 76e6ab37fc0cea11e1fd16704dd66639f5673460 Mon Sep 17 00:00:00
> > 2001 From: Bryce Harrington <[email protected]>
> > Date: Thu, 2 Apr 2015 19:30:22 -0700
> > Subject: [PATCH] tests: Integrate overrides for ivi shell
> >
> > Signed-off-by: Bryce Harrington <[email protected]>
> > ---
> > tests/weston-tests-env | 23 +++++++++++++++--------
> > 1 file changed, 15 insertions(+), 8 deletions(-)
> >
> > diff --git a/tests/weston-tests-env b/tests/weston-tests-env
> > index c0a211e..8055939 100755
> > --- a/tests/weston-tests-env
> > +++ b/tests/weston-tests-env
> > @@ -21,15 +21,10 @@ BACKEND=${BACKEND:-headless-backend.so}
> > MODDIR=$abs_builddir/.libs
> > TEST_MODULE=$MODDIR/${TEST_FILE/.la/.so}
> > SHELL_PLUGIN=$MODDIR/desktop-shell.so
> > +TEST_PLUGIN=$MODDIR/weston-test.so
> > XWAYLAND_PLUGIN=$MODDIR/xwayland.so
> > -
> > -if [[ ${TEST_MODULE} != *.so ]]; then
> > - export WESTON_TEST_CLIENT_PATH=$abs_builddir/$TEST_FILE
> > - client=$($WESTON_TEST_CLIENT_PATH --params)
> > - TEST_MODULE=$MODDIR/weston-test.so
> > -fi
> > -
> > CONFIG_FILE="${TEST_NAME}.ini"
> > +
> > if [ -e "${abs_builddir}/${CONFIG_FILE}" ]; then
> > CONFIG="--config=${abs_builddir}/${CONFIG_FILE}"
> > elif [ -e "${abs_top_srcdir}/tests/${CONFIG_FILE}" ]; then
> > @@ -38,11 +33,23 @@ else
> > CONFIG="--no-config"
> > fi
> >
> > +if [[ ${TEST_MODULE} != *.so ]]; then
> > + export WESTON_TEST_CLIENT_PATH=$abs_builddir/$TEST_FILE
> > + client=$($WESTON_TEST_CLIENT_PATH --params)
> > + MODULES=$TEST_PLUGIN,$XWAYLAND_PLUGIN
> > +fi
> > +
> > +if [[ ${TEST_MODULE} == ivi-* ]]; then
> > + SHELL_PLUGIN=$MODDIR/ivi-shell.so
> > + CONFIG="--config=${abs_builddir}/tests/weston-ivi.ini"
> > + MODULES=$TEST_PLUGIN
> > +fi
> > +
> > export WESTON_BUILD_DIR=$abs_builddir
> > $WESTON \
> > --shell=$SHELL_PLUGIN \
> > --socket=test-${TEST_NAME} \
> > - --modules=$TEST_MODULE,$XWAYLAND_PLUGIN \
> > + --modules=$MODULES \
> > --backend=$MODDIR/$BACKEND \
> > --log="$SERVERLOG" \
> > $CONFIG \
> > --
> > 1.9.1
> >
>
> Is this the diff needed to support the ivi tests? Not bad, but it's
> not handling the ivi-*.so case properly AFAICS: --modules vs.
> --ivi-module.
>
> My point is, sure you can make this style work, but is it any more
> clear than the big switch-case?
Yes, to me it is.
> To me this looks more fragmented, while it does avoid copying
> errors by reducing redundant lines. Set a variable, then add an
> exception to set it to something else, then add another exception
> on another condition... I think it becomes hard to follow what will
> eventually be executed, because the reader needs to parse through
> all conditionals to see what a single case does.
>
> Isn't the point of this refactoring to improve readability? I'm not
> completely sure it achieves that.
Well do whatever you want to do then.
Bryce
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
