Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

[Michael S. Tsirkin]

On Tue, Nov 06, 2018 at 04:15:03PM +0100, Philippe Mathieu-Daudé wrote:
> On 6/11/18 15:13, Michael S. Tsirkin wrote:
> > On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 5/11/18 19:14, Michael S. Tsirkin wrote:
> > > > From: Peter Maydell <address@hidden>
> > > > 
> > > > Currently tests/hex-loader-check-data contains data files used
> > > > by the hexloader-test, and configure individually symlinks those
> > > > data files into the build directory using a wildcard.
> > > > 
> > > > Using a wildcard like this is a bad idea, because if a new
> > > > data file is added, nothing causes configure to be rerun,
> > > > and so no symlink is added for the new file. This can cause
> > > > tests to spuriously fail when they can't find their data.
> > > > Instead, it's better to symlink an entire directory of
> > > > data files. We already have such a directory: tests/data.
> > > > 
> > > > Move the data files from tests/hex-loader-check-data/ to
> > > > tests/data/hex-loader/, and remove the unnecessary symlinking.
> > > > 
> > > > Signed-off-by: Peter Maydell <address@hidden>
> > > 
> > > I reviewed/tested this patch too.
> > 
> > 
> > Thanks a lot Philippe!
> > It is unfortunately too late to update this patch info in git
> > commit history, however your help is still greatly appreciated!
> 
> No worry, I'm not mad at all, but there might be an issue in your git PR
> workflow, this series also missed your maintainer S-o-b.

It's just that I could not figure out the failures that were blocking
the pull, so when I saw that Peter finally posted the fix I rushed to
merge and test it and didn't look for any acks.  My mistake, sorry about
that. That's also why I forgot to sign it.

> Peter: Can you add a such check in your scripts? (during next merge window,
> no hurry).
> 
> Rather than your scripts, this should be in scripts a maintainer can run
> locally, such ./scripts/checkpatch.pl --maintainer or
> ./scripts/checkseries.xx.
> 
> > 
> > 
> > > > ---
> > > >    configure                                                 | 4 ----
> > > >    tests/hexloader-test.c                                    | 2 +-
> > > >    MAINTAINERS                                               | 2 +-
> > > >    tests/{hex-loader-check-data => data/hex-loader}/test.hex | 0
> > > >    4 files changed, 2 insertions(+), 6 deletions(-)
> > > >    rename tests/{hex-loader-check-data => data/hex-loader}/test.hex 
> > > > (100%)
> > > > 
> > > > diff --git a/configure b/configure
> > > > index 895b7483b8..bfdca8b814 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -7421,10 +7421,6 @@ for bios_file in \
> > > >    do
> > > >        FILES="$FILES pc-bios/$(basename $bios_file)"
> > > >    done
> > > > -for test_file in $(find $source_path/tests/hex-loader-check-data -type 
> > > > f)
> > > > -do
> > > > -    FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed 
> > > > -e 's/.*hex-loader-check-data//')"
> > > > -done
> > > >    mkdir -p $DIRS
> > > >    for f in $FILES ; do
> > > >        if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; 
> > > > then
> > > > diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
> > > > index b653d44ba1..834ed52c22 100644
> > > > --- a/tests/hexloader-test.c
> > > > +++ b/tests/hexloader-test.c
> > > > @@ -23,7 +23,7 @@ static void hex_loader_test(void)
> > > >        const unsigned int base_addr = 0x00010000;
> > > >        QTestState *s = qtest_initf(
> > > > -        "-M vexpress-a9 -nographic -device 
> > > > loader,file=tests/hex-loader-check-data/test.hex");
> > > > +        "-M vexpress-a9 -nographic -device 
> > > > loader,file=tests/data/hex-loader/test.hex");
> > > >        for (i = 0; i < 256; ++i) {
> > > >            uint8_t val = qtest_readb(s, base_addr + i);
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 98a1856afc..cfabc14b59 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -1370,7 +1370,7 @@ Intel Hexadecimal Object File Loader
> > > >    M: Su Hang <address@hidden>
> > > >    S: Maintained
> > > >    F: tests/hexloader-test.c
> > > > -F: tests/hex-loader-check-data/test.hex
> > > > +F: tests/data/hex-loader/test.hex
> > > >    CHRP NVRAM
> > > >    M: Thomas Huth <address@hidden>
> > > > diff --git a/tests/hex-loader-check-data/test.hex 
> > > > b/tests/data/hex-loader/test.hex
> > > > similarity index 100%
> > > > rename from tests/hex-loader-check-data/test.hex
> > > > rename to tests/data/hex-loader/test.hex
> > > >