* [Qemu-devel] check-qdict and check-qjson fail @ 2011-08-26 16:48 Jan Kiszka 2011-08-26 17:43 ` Luiz Capitulino 0 siblings, 1 reply; 7+ messages in thread From: Jan Kiszka @ 2011-08-26 16:48 UTC (permalink / raw) To: Luiz Capitulino, Anthony Liguori; +Cc: qemu-devel Hi all, current master has some qregressions: # ./check-qdict Running suite(s): QDict test-suite 93%: Checks: 16, Failures: 1, Errors: 0 check-qdict.c:289:F:Stress Test:qdict_stress_test:0: Assertion 'test_file != NULL' failed # ./check-qjson Running suite(s): QJSON test-suite 95%: Checks: 21, Failures: 1, Errors: 0 check-qjson.c:59:F:String Literals:escaped_string:0: / != \/ Already known? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] check-qdict and check-qjson fail 2011-08-26 16:48 [Qemu-devel] check-qdict and check-qjson fail Jan Kiszka @ 2011-08-26 17:43 ` Luiz Capitulino 2011-08-26 17:52 ` [Qemu-devel] [PATCH] Fix qjson test of solidus encoding Jan Kiszka 0 siblings, 1 reply; 7+ messages in thread From: Luiz Capitulino @ 2011-08-26 17:43 UTC (permalink / raw) To: Jan Kiszka; +Cc: Anthony Liguori, qemu-devel, mdroth On Fri, 26 Aug 2011 18:48:52 +0200 Jan Kiszka <jan.kiszka@siemens.com> wrote: > Hi all, > > current master has some qregressions: > > # ./check-qdict > Running suite(s): QDict test-suite > 93%: Checks: 16, Failures: 1, Errors: 0 > check-qdict.c:289:F:Stress Test:qdict_stress_test:0: Assertion 'test_file != NULL' failed I guess you build qemu from a different directory, right? This test only works from the source dir. I'll fix it (actually I thought I had already fixed it!). > # ./check-qjson > Running suite(s): QJSON test-suite > 95%: Checks: 21, Failures: 1, Errors: 0 > check-qjson.c:59:F:String Literals:escaped_string:0: / != \/ I found this problem while reading the parser code and added the test so that we don't forget fixing it. However, at the time I looked at the issue, I didn't know how to fix it. Michael, would you mind to take a look at it? > > Already known? > > Jan > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH] Fix qjson test of solidus encoding 2011-08-26 17:43 ` Luiz Capitulino @ 2011-08-26 17:52 ` Jan Kiszka 2011-08-26 17:58 ` Luiz Capitulino 2011-08-29 14:19 ` Michael Roth 0 siblings, 2 replies; 7+ messages in thread From: Jan Kiszka @ 2011-08-26 17:52 UTC (permalink / raw) To: Luiz Capitulino; +Cc: Anthony Liguori, qemu-devel, mdroth@linux.vnet.ibm.com On 2011-08-26 19:43, Luiz Capitulino wrote: > On Fri, 26 Aug 2011 18:48:52 +0200 > Jan Kiszka <jan.kiszka@siemens.com> wrote: > >> Hi all, >> >> current master has some qregressions: >> >> # ./check-qdict >> Running suite(s): QDict test-suite >> 93%: Checks: 16, Failures: 1, Errors: 0 >> check-qdict.c:289:F:Stress Test:qdict_stress_test:0: Assertion 'test_file != NULL' failed > > I guess you build qemu from a different directory, right? This test only > works from the source dir. I'll fix it (actually I thought I had already fixed it!). Yes, found that reason already. Works fine when fed with the test data as expected. > >> # ./check-qjson >> Running suite(s): QJSON test-suite >> 95%: Checks: 21, Failures: 1, Errors: 0 >> check-qjson.c:59:F:String Literals:escaped_string:0: / != \/ > > I found this problem while reading the parser code and added the test > so that we don't forget fixing it. However, at the time I looked at the > issue, I didn't know how to fix it. > > Michael, would you mind to take a look at it? After reading the spec I tend to think it's a test bug: -------8<------- "\/" is supposed to be decoded as "/", but there is no need to encode "/" via escape. Fix the existing test and add a second one expressing this. Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- check-qjson.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/check-qjson.c b/check-qjson.c index 64fcdcb..36d4ac2 100644 --- a/check-qjson.c +++ b/check-qjson.c @@ -33,7 +33,8 @@ START_TEST(escaped_string) { "\"\\n\"", "\n" }, { "\"\\r\"", "\r" }, { "\"\\t\"", "\t" }, - { "\"\\/\"", "\\/" }, + { "\"/\"", "/" }, + { "\"\\/\"", "/", .skip = 1 }, { "\"\\\\\"", "\\" }, { "\"\\\"\"", "\"" }, { "\"hello world \\\"embedded string\\\"\"", -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix qjson test of solidus encoding 2011-08-26 17:52 ` [Qemu-devel] [PATCH] Fix qjson test of solidus encoding Jan Kiszka @ 2011-08-26 17:58 ` Luiz Capitulino 2011-08-26 17:58 ` Luiz Capitulino 2011-08-29 14:19 ` Michael Roth 1 sibling, 1 reply; 7+ messages in thread From: Luiz Capitulino @ 2011-08-26 17:58 UTC (permalink / raw) To: Jan Kiszka; +Cc: Anthony Liguori, qemu-devel, mdroth@linux.vnet.ibm.com On Fri, 26 Aug 2011 19:52:11 +0200 Jan Kiszka <jan.kiszka@siemens.com> wrote: > On 2011-08-26 19:43, Luiz Capitulino wrote: > > On Fri, 26 Aug 2011 18:48:52 +0200 > > Jan Kiszka <jan.kiszka@siemens.com> wrote: > > > >> Hi all, > >> > >> current master has some qregressions: > >> > >> # ./check-qdict > >> Running suite(s): QDict test-suite > >> 93%: Checks: 16, Failures: 1, Errors: 0 > >> check-qdict.c:289:F:Stress Test:qdict_stress_test:0: Assertion 'test_file != NULL' failed > > > > I guess you build qemu from a different directory, right? This test only > > works from the source dir. I'll fix it (actually I thought I had already fixed it!). > > Yes, found that reason already. Works fine when fed with the test data > as expected. Fix below. > >> # ./check-qjson > >> Running suite(s): QJSON test-suite > >> 95%: Checks: 21, Failures: 1, Errors: 0 > >> check-qjson.c:59:F:String Literals:escaped_string:0: / != \/ > > > > I found this problem while reading the parser code and added the test > > so that we don't forget fixing it. However, at the time I looked at the > > issue, I didn't know how to fix it. > > > > Michael, would you mind to take a look at it? > > After reading the spec I tend to think it's a test bug: Seems good to me. What do you think Anthony/Michael? > > -------8<------- > > "\/" is supposed to be decoded as "/", but there is no need to encode > "/" via escape. Fix the existing test and add a second one expressing > this. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > check-qjson.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/check-qjson.c b/check-qjson.c > index 64fcdcb..36d4ac2 100644 > --- a/check-qjson.c > +++ b/check-qjson.c > @@ -33,7 +33,8 @@ START_TEST(escaped_string) > { "\"\\n\"", "\n" }, > { "\"\\r\"", "\r" }, > { "\"\\t\"", "\t" }, > - { "\"\\/\"", "\\/" }, > + { "\"/\"", "/" }, > + { "\"\\/\"", "/", .skip = 1 }, > { "\"\\\\\"", "\\" }, > { "\"\\\"\"", "\"" }, > { "\"hello world \\\"embedded string\\\"\"", diff --git a/configure b/configure index 1340c33..7fab4de 100755 --- a/configure +++ b/configure @@ -3601,7 +3601,7 @@ DIRS="$DIRS roms/seabios roms/vgabios" DIRS="$DIRS fsdev ui" DIRS="$DIRS qapi" DIRS="$DIRS qga" -FILES="Makefile tests/Makefile" +FILES="Makefile tests/Makefile qdict-test-data.txt" FILES="$FILES tests/cris/Makefile tests/cris/.gdbinit" FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps" FILES="$FILES pc-bios/spapr-rtas/Makefile" ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix qjson test of solidus encoding 2011-08-26 17:58 ` Luiz Capitulino @ 2011-08-26 17:58 ` Luiz Capitulino 0 siblings, 0 replies; 7+ messages in thread From: Luiz Capitulino @ 2011-08-26 17:58 UTC (permalink / raw) To: Luiz Capitulino Cc: Jan Kiszka, Anthony Liguori, qemu-devel, mdroth@linux.vnet.ibm.com On Fri, 26 Aug 2011 14:58:15 -0300 Luiz Capitulino <lcapitulino@redhat.com> wrote: > On Fri, 26 Aug 2011 19:52:11 +0200 > Jan Kiszka <jan.kiszka@siemens.com> wrote: > > > On 2011-08-26 19:43, Luiz Capitulino wrote: > > > On Fri, 26 Aug 2011 18:48:52 +0200 > > > Jan Kiszka <jan.kiszka@siemens.com> wrote: > > > > > >> Hi all, > > >> > > >> current master has some qregressions: > > >> > > >> # ./check-qdict > > >> Running suite(s): QDict test-suite > > >> 93%: Checks: 16, Failures: 1, Errors: 0 > > >> check-qdict.c:289:F:Stress Test:qdict_stress_test:0: Assertion 'test_file != NULL' failed > > > > > > I guess you build qemu from a different directory, right? This test only > > > works from the source dir. I'll fix it (actually I thought I had already fixed it!). > > > > Yes, found that reason already. Works fine when fed with the test data > > as expected. > > Fix below. > > > >> # ./check-qjson > > >> Running suite(s): QJSON test-suite > > >> 95%: Checks: 21, Failures: 1, Errors: 0 > > >> check-qjson.c:59:F:String Literals:escaped_string:0: / != \/ > > > > > > I found this problem while reading the parser code and added the test > > > so that we don't forget fixing it. However, at the time I looked at the > > > issue, I didn't know how to fix it. > > > > > > Michael, would you mind to take a look at it? > > > > After reading the spec I tend to think it's a test bug: > > Seems good to me. What do you think Anthony/Michael? Read: please ack :) > > > > > -------8<------- > > > > "\/" is supposed to be decoded as "/", but there is no need to encode > > "/" via escape. Fix the existing test and add a second one expressing > > this. > > > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > > --- > > check-qjson.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/check-qjson.c b/check-qjson.c > > index 64fcdcb..36d4ac2 100644 > > --- a/check-qjson.c > > +++ b/check-qjson.c > > @@ -33,7 +33,8 @@ START_TEST(escaped_string) > > { "\"\\n\"", "\n" }, > > { "\"\\r\"", "\r" }, > > { "\"\\t\"", "\t" }, > > - { "\"\\/\"", "\\/" }, > > + { "\"/\"", "/" }, > > + { "\"\\/\"", "/", .skip = 1 }, > > { "\"\\\\\"", "\\" }, > > { "\"\\\"\"", "\"" }, > > { "\"hello world \\\"embedded string\\\"\"", > > diff --git a/configure b/configure > index 1340c33..7fab4de 100755 > --- a/configure > +++ b/configure > @@ -3601,7 +3601,7 @@ DIRS="$DIRS roms/seabios roms/vgabios" > DIRS="$DIRS fsdev ui" > DIRS="$DIRS qapi" > DIRS="$DIRS qga" > -FILES="Makefile tests/Makefile" > +FILES="Makefile tests/Makefile qdict-test-data.txt" > FILES="$FILES tests/cris/Makefile tests/cris/.gdbinit" > FILES="$FILES pc-bios/optionrom/Makefile pc-bios/keymaps" > FILES="$FILES pc-bios/spapr-rtas/Makefile" ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix qjson test of solidus encoding 2011-08-26 17:52 ` [Qemu-devel] [PATCH] Fix qjson test of solidus encoding Jan Kiszka 2011-08-26 17:58 ` Luiz Capitulino @ 2011-08-29 14:19 ` Michael Roth 2011-08-29 16:58 ` Luiz Capitulino 1 sibling, 1 reply; 7+ messages in thread From: Michael Roth @ 2011-08-29 14:19 UTC (permalink / raw) To: Jan Kiszka; +Cc: Anthony Liguori, qemu-devel, Luiz Capitulino On 08/26/2011 12:52 PM, Jan Kiszka wrote: > On 2011-08-26 19:43, Luiz Capitulino wrote: >> On Fri, 26 Aug 2011 18:48:52 +0200 >> Jan Kiszka<jan.kiszka@siemens.com> wrote: >> >>> Hi all, >>> >>> current master has some qregressions: >>> >>> # ./check-qdict >>> Running suite(s): QDict test-suite >>> 93%: Checks: 16, Failures: 1, Errors: 0 >>> check-qdict.c:289:F:Stress Test:qdict_stress_test:0: Assertion 'test_file != NULL' failed >> >> I guess you build qemu from a different directory, right? This test only >> works from the source dir. I'll fix it (actually I thought I had already fixed it!). > > Yes, found that reason already. Works fine when fed with the test data > as expected. > >> >>> # ./check-qjson >>> Running suite(s): QJSON test-suite >>> 95%: Checks: 21, Failures: 1, Errors: 0 >>> check-qjson.c:59:F:String Literals:escaped_string:0: / != \/ >> >> I found this problem while reading the parser code and added the test >> so that we don't forget fixing it. However, at the time I looked at the >> issue, I didn't know how to fix it. >> >> Michael, would you mind to take a look at it? > > After reading the spec I tend to think it's a test bug: > > -------8<------- > > "\/" is supposed to be decoded as "/", but there is no need to encode > "/" via escape. Fix the existing test and add a second one expressing > this. > > Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> > --- > check-qjson.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/check-qjson.c b/check-qjson.c > index 64fcdcb..36d4ac2 100644 > --- a/check-qjson.c > +++ b/check-qjson.c > @@ -33,7 +33,8 @@ START_TEST(escaped_string) > { "\"\\n\"", "\n" }, > { "\"\\r\"", "\r" }, > { "\"\\t\"", "\t" }, > - { "\"\\/\"", "\\/" }, > + { "\"/\"", "/" }, > + { "\"\\/\"", "/", .skip = 1 }, > { "\"\\\\\"", "\\" }, > { "\"\\\"\"", "\"" }, > { "\"hello world \\\"embedded string\\\"\"", Acked-by: Michael Roth <mdroth@linux.vnet.ibm.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] Fix qjson test of solidus encoding 2011-08-29 14:19 ` Michael Roth @ 2011-08-29 16:58 ` Luiz Capitulino 0 siblings, 0 replies; 7+ messages in thread From: Luiz Capitulino @ 2011-08-29 16:58 UTC (permalink / raw) To: Michael Roth; +Cc: Jan Kiszka, Anthony Liguori, qemu-devel On Mon, 29 Aug 2011 09:19:30 -0500 Michael Roth <mdroth@linux.vnet.ibm.com> wrote: > On 08/26/2011 12:52 PM, Jan Kiszka wrote: > > On 2011-08-26 19:43, Luiz Capitulino wrote: > >> On Fri, 26 Aug 2011 18:48:52 +0200 > >> Jan Kiszka<jan.kiszka@siemens.com> wrote: > >> > >>> Hi all, > >>> > >>> current master has some qregressions: > >>> > >>> # ./check-qdict > >>> Running suite(s): QDict test-suite > >>> 93%: Checks: 16, Failures: 1, Errors: 0 > >>> check-qdict.c:289:F:Stress Test:qdict_stress_test:0: Assertion 'test_file != NULL' failed > >> > >> I guess you build qemu from a different directory, right? This test only > >> works from the source dir. I'll fix it (actually I thought I had already fixed it!). > > > > Yes, found that reason already. Works fine when fed with the test data > > as expected. > > > >> > >>> # ./check-qjson > >>> Running suite(s): QJSON test-suite > >>> 95%: Checks: 21, Failures: 1, Errors: 0 > >>> check-qjson.c:59:F:String Literals:escaped_string:0: / != \/ > >> > >> I found this problem while reading the parser code and added the test > >> so that we don't forget fixing it. However, at the time I looked at the > >> issue, I didn't know how to fix it. > >> > >> Michael, would you mind to take a look at it? > > > > After reading the spec I tend to think it's a test bug: > > > > -------8<------- > > > > "\/" is supposed to be decoded as "/", but there is no need to encode > > "/" via escape. Fix the existing test and add a second one expressing > > this. > > > > Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com> > > --- > > check-qjson.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/check-qjson.c b/check-qjson.c > > index 64fcdcb..36d4ac2 100644 > > --- a/check-qjson.c > > +++ b/check-qjson.c > > @@ -33,7 +33,8 @@ START_TEST(escaped_string) > > { "\"\\n\"", "\n" }, > > { "\"\\r\"", "\r" }, > > { "\"\\t\"", "\t" }, > > - { "\"\\/\"", "\\/" }, > > + { "\"/\"", "/" }, > > + { "\"\\/\"", "/", .skip = 1 }, > > { "\"\\\\\"", "\\" }, > > { "\"\\\"\"", "\"" }, > > { "\"hello world \\\"embedded string\\\"\"", > > Acked-by: Michael Roth <mdroth@linux.vnet.ibm.com> Applied to the qmp branch, thanks. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-08-29 16:58 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-08-26 16:48 [Qemu-devel] check-qdict and check-qjson fail Jan Kiszka 2011-08-26 17:43 ` Luiz Capitulino 2011-08-26 17:52 ` [Qemu-devel] [PATCH] Fix qjson test of solidus encoding Jan Kiszka 2011-08-26 17:58 ` Luiz Capitulino 2011-08-26 17:58 ` Luiz Capitulino 2011-08-29 14:19 ` Michael Roth 2011-08-29 16:58 ` Luiz Capitulino
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).