Re: [PATCH v4 2/2] tests/unit: Add an unit test for smp parsing

[Thomas Huth]

On 09/11/2021 13.18, wangyanan (Y) wrote:
> Hi,
>
> On 2021/11/9 17:36, Philippe Mathieu-Daudé wrote:
> > Hi,
> >
> > On 10/28/21 17:09, Philippe Mathieu-Daudé wrote:
> > > From: Yanan Wang <wangyanan55@huawei.com>
> > >
> > > Now that we have a generic parser smp_parse(), let's add an unit
> > > test for the code. All possible valid/invalid SMP configurations
> > > that the user can specify are covered.
> > >
> > > Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> > > Reviewed-by: Andrew Jones <drjones@redhat.com>
> > > Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > Message-Id: <20211026034659.22040-3-wangyanan55@huawei.com>
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > ---
> > >   tests/unit/test-smp-parse.c | 594 ++++++++++++++++++++++++++++++++++++
> > >   MAINTAINERS                 |   1 +
> > >   tests/unit/meson.build      |   1 +
> > >   3 files changed, 596 insertions(+)
> > >   create mode 100644 tests/unit/test-smp-parse.c
> > Could you have a look at this test failure please?
> > https://cirrus-ci.com/task/5823855357853696
> >
> > MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> > G_TEST_SRCDIR=C:/Users/ContainerAdministrator/AppData/Local/Temp/cirrus-ci-build/tests/unit 
> >
> > G_TEST_BUILDDIR=C:/Users/ContainerAdministrator/AppData/Local/Temp/cirrus-ci-build/build/tests/unit 
> >
> > tests/unit/test-smp-parse.exe --tap -k
> > Test smp_parse failed!
> > Input configuration: (SMPConfiguration) {
> >      .has_cpus    =  true, cpus    = 1,
> >      .has_sockets = false, sockets = 0,
> >      .has_dies    = false, dies    = 0,
> >      .has_cores   = false, cores   = 0,
> >      .has_threads = false, threads = 0,
> >      .has_maxcpus = false, maxcpus = 0,
> > }
> > Should be valid: no
> > Expected error report: Invalid SMP CPUs 1. The min CPUs supported by
> > machine '(null)' is 2
> > Result is valid: no
> > Output error report: Invalid SMP CPUs 1. The min CPUs supported by
> > machine '(NULL)' is 2
> > ERROR test-smp-parse - too few tests run (expected 2, got 0)
> > make: *** [Makefile.mtest:576: run-test-70] Error 1
> >
> >
> > .
> Obviously, the name string for the tested machine type in cirrus-ci is "NULL",
> while the expected name string is "null". It was also "null" when running on
> my Arm64 platform locally.
>
> Anyway, I shouldn't have hardcoded this expected error message. I can
> send a fix patch to make it more flexible then more stable. :)

Yes, please do so. The "(null)"/"(NULL)" obviously depends on the libc that 
is used, so we should not rely on the correct spelling of the string in the 
tests here.

 Thomas